Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bikeshedding force_in(Unit) #534

Open
mpusz opened this issue Dec 23, 2023 · 10 comments
Open

Bikeshedding force_in(Unit) #534

mpusz opened this issue Dec 23, 2023 · 10 comments
Labels
design Design-related discussion iso The ISO C++ Committee related work question Further information is requested

Comments

@mpusz
Copy link
Owner

mpusz commented Dec 23, 2023

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 mpusz added question Further information is requested design Design-related discussion labels Dec 23, 2023
@mpusz
Copy link
Owner Author

mpusz commented Dec 23, 2023

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

@chiphogg
Copy link
Collaborator

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... 🤔

@mpusz
Copy link
Owner Author

mpusz commented Dec 23, 2023

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Owner Author

mpusz commented Dec 26, 2023

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

@mpusz
Copy link
Owner Author

mpusz commented Dec 26, 2023

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
Copy link

Spammed commented Dec 26, 2023

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
Copy link
Owner Author

mpusz commented Dec 27, 2023

"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
Copy link

Spammed commented Dec 27, 2023

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'

@mpusz mpusz added the iso The ISO C++ Committee related work label Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design-related discussion iso The ISO C++ Committee related work question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants