mpusz / mp-units

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

Bikeshedding `force_in(Unit)` #534

Open mpusz opened 6 months ago

mpusz commented 6 months ago

As we discussed before, the spelling of distance.force_in(si::metre) may be confusing. Also, the C++ standard does not use "force" word anywhere to force a conversion.

AU library is using coerse_in(Unit) for that reason but I personally do not think it helps here.

I just thought about renaming it to cast_to(si::metre). Lets see some examples:

quantity q1 = 42 * m;
quantity q2 = q1.in(mm);
quantity q3 = q1.cast_to(km);
quantity q4 = q1.numerical_value_in(mm);
quantity q5 = q1.cast_numerical_value_to(km);
quantity q6 = q1.numerical_value_cast_to(km);

What do you think of the above? Which one looks better to you, q5 or q6?

mpusz commented 6 months ago

The above naming is also more consistent with the value_cast<Unit>(q) that does the same thing.

chiphogg commented 6 months ago

I could see this becoming idiomatic. One downside may be that we're changing the verb, from "in" to "to". The reason to prefer keeping the verb the same is that the API names become easier to compose, easier to "guess". That said, I think cast_to probably sounds better on its own than cast_in... :thinking:

mpusz commented 6 months ago

Yes, I agree that naming is hard ;-)

The first question is, do we agree that cast_to is a better choice than force_in?

The second one is if we have any better candidates.

chiphogg commented 6 months ago

I think cast_to is better than force_in, yes. I'm starting to suspect that the global optimum might be .to and .cast_to, but changing .in to .to would be a monumental shift. I think .cast_in/.in is probably slightly better than .cast_to/.in.

JohelEGP commented 6 months ago

I think .cast_in/.in is probably slightly better than .cast_to/.in.

That makes sense. "To" usually denotes the target type. In this case, we're not changing the type, but the units.

mpusz commented 6 months ago

Please note that at some point, we may also have .cast_to<int>(). cast_in<int>() does not look that nice anymore.

mpusz commented 6 months ago

I thought about .cast_in() as well but then the problem comes with cast_numerical_value_in() or numerical_value_cast_in() where both seem to not read right. cast_numerical_value_to() looks better for me.

Spammed commented 6 months ago

As I (not native speaker) understand it, 'cast' denotes a operation, so 'cast_to' sounds more correct than 'cast_in', which might better mean 'casted_in'.

Are 'stored_in', 'kept_in' or 'hold_in' alternatives?

mpusz commented 6 months ago

"Stored", "kept", or "hold" are not a valid solution here. We need two options for conversion. One that means 'convert safely' (e.g., no data truncation), and for that, we have .in() now. The second one means 'force conversion even if unsafe', and for that, we have now .force_in() now. I propose to change the latter to cast_to() or similar as "casting" in C++ means a forced conversion. The Au library uses .coerse_in(), which is also an option, but I personally am not a fan of it.

Spammed commented 6 months ago

Ah ok, thanks for clarification. So, 'unsafe_cast_to' is a little bit too unhandy? 'forced_to' does not pose any risk of confusion for me. PS: You mean 'coerce_in'