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

Unitless types #106

Closed
DavidTheLost opened this issue Apr 11, 2022 · 10 comments
Closed

Unitless types #106

DavidTheLost opened this issue Apr 11, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@DavidTheLost
Copy link

centi_metre_t a = 15;
square_metre_t c = a / a; // This is not square meters!!!
CHECK_ALMOST_EQUAL(c.value(), 1.0F, 1E-6);

The units of 'c' are not square metres! In fact 'c' does not have units. So why does this compile?

@ypearson
Copy link

You sure a/a doesn't get implicitly converted to 1 m^2 ?

@DavidTheLost
Copy link
Author

DavidTheLost commented Apr 11, 2022 via email

@ypearson
Copy link

Yeah, I agree. However, its the same as this:
square_metre_t c = 1
Does the above compile? 1 is unitless too

@DavidTheLost
Copy link
Author

DavidTheLost commented Apr 11, 2022 via email

@ypearson
Copy link

Maybe one solution is to not allow implicit conversion
Idea:
metre_t d1 = 5; //does not compile
metre_t d2 = 5*1_m; //does compile

I'm not the author, but I'm interested in this discussion.

@DavidTheLost
Copy link
Author

DavidTheLost commented Apr 12, 2022 via email

@bernedom
Copy link
Owner

As you guessed, this happens because of the implicit conversion.

metre_t x = 1
will essentially do the same as
metre_t x{1}

What happens in the example is that a / a results in a scalar which is then converted back to square_metre_t

centi_metre_t a = 15;
square_metre_t c = a / a; // This is not square meters!!

So if you change the example to use auto c = a/a then c will be of a scalar/primitive type. This is also why I personally think that using auto for all types which are the result of a computation or a function is good practice, but I see how this can become confusing.

Marking the constructors as explicit might prevent assigning the scalar directly, but I will have to check this and also see what else is affected by this.

@bernedom bernedom self-assigned this Apr 12, 2022
@bernedom bernedom added the enhancement New feature or request label Apr 12, 2022
@bernedom
Copy link
Owner

I solved the issue by marking the constructor that takes a primitive for the units to explicit

It is in the following branch: https://github.com/bernedom/SI/tree/feature/no-implicit-conversion-on-assignment-operator

Any feedback if the behavior is as expected would be great.

@bernedom
Copy link
Owner

This will be in release 2.5.0

@DavidTheLost
Copy link
Author

DavidTheLost commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants