mpusz / mp-units

The quantities and units library for C++
https://mpusz.github.io/mp-units/
MIT License
1.07k stars 85 forks source link

`make_quantity` or a constructor with two arguments? #521

Closed mpusz closed 10 months ago

mpusz commented 10 months ago

Right now, the library exposes make_quantity and make_quantity_point factory functions to create a quantity without the use of a multiply or plus syntax. They are useful when a user does not like the alternative syntaxes or can't use them because of some ambiguities in the interface (e.g. in the case of linear algebra representation types).

However, those need to be friends of our class templates, which complicates the design and introduces some awkwardness.

Maybe we should just use constructors with two arguments (value and unit)? The drawback of such a solution is that, in many cases, we will need to copy the same information twice:

quantity<si::metre, int> q(42, si::metre);

However, if we want to use a different initializer or stick with CTAD, then it is not the issue anymore. Here are some use cases for all of the options:

void foo(quantity<si::metre, int> q);

foo(42 * si::metre);
foo(make_quantity<si::metre>(42));
foo({42, si::metre});          // assuming implicit constructor
foo(quantity{42, si::metre});  // assuming explicit constructor

quantity<si::metre, int> q1 = 42 * si::metre;
quantity<si::metre, int> q2 = make_quantity<si::metre>(42);
quantity<si::metre, int> q3 = {42, si::metre};  // assuming implicit constructor
quantity<si::metre, int> q4(42, si::metre);     // assuming explicit constructor

quantity<si::metre, int> q5 = 42 * si::kilo<si::metre>;
quantity<si::metre, int> q6 = make_quantity<si::kilo<si::metre>>(42);
quantity<si::metre, int> q7 = {42, si::kilo<si::metre>};  // assuming implicit constructor
quantity<si::metre, int> q8(42, si::kilo<si::metre>);     // assuming explicit constructor

quantity q9 = 42 * m;
quantity q10 = make_quantity<si::metre>(42);
quantity q11 = quantity{42, si::metre};

Should such a constructor be explicit or implicit?

Please note that at least in the current library implementation, we will need to use friends as well to implement this.

JohelEGP commented 10 months ago

We should default to explicit unless proven otherwise. But if the constructor behaves the same as quantity<U1, R1>(quantity{R2(), U2}), I don't see why it can't be implicit in all cases. It'll already be as safe, and terser.

mpusz commented 10 months ago

But if the constructor behaves the same as quantity<U1, R1>(quantity{R2(), U2}), I don't see why it can't be implicit in all cases.

Yes, this is precisely the case, and that is the reason why we need a friend constructor or a helper function to make it compile.

mpusz commented 10 months ago

But do you think that a two-argument constructor is a better solution than a factory function here?

JohelEGP commented 10 months ago

I think the constructor is better. make_quantity is really special in that it requires friendship, but only because it can't be implemented in terms of a constructor. I imagine the optimization by std::make_shared can be implemented similarly with friendship, but make_quantity doesn't actually provide any benefit over a constructor (of which there's none).