bernedom / SI

A header only C++ library that provides type safety and user defined literals for physical units
https://si.dominikberner.ch/doc/
MIT License
486 stars 40 forks source link

Unitless types #106

Closed DavidTheLost closed 2 years ago

DavidTheLost commented 2 years ago

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 commented 2 years ago

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

DavidTheLost commented 2 years ago

Hi,

It does get converted to m^2. But if you divide 15m by 15m you should get 1 not 1m^2.

David

From: ypearson @. Sent: 11 April 2022 15:01 To: bernedom/SI @.> Cc: David Orchard (SXS UK) @.>; Author @.> Subject: Re: [bernedom/SI] Unitless types (Issue #106)

EXTERNAL EMAIL: Remember the 9 Phishing Indicators

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

— Reply to this email directly, view it on GitHubhttps://github.com/bernedom/SI/issues/106#issuecomment-1095090615, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYVEJ4C75I3F2VFDW7WX4W3VEQWBJANCNFSM5TDHY33Q. You are receiving this because you authored the thread.Message ID: @.***>


Spirax-Sarco Engineering Plc. This e-mail has been scanned for viruses by Cisco Cloud Email Security.

ypearson commented 2 years ago

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

DavidTheLost commented 2 years ago

I see what you are getting at:

square_metre_t<float> c1 = 1;   // This is not square meters!!!

This does compile. You are saying 1 is a square meter. However, when the value is the result of a computation you are relying on the engineer not making a mistake and messing up his units.

square_metre_t< float > c2 = 1_m2;   // This is square meters!!!
square_metre_t< float > c3 = 1;      // This is not square meters!!!

using m2 = 1_m2;
float data = 15.0F;
square_metre_t< float > c3 = data * 1_m2;   // This is square meters!!!
square_metre_t< float > c4 = data * m2;     // Nicer!!!

From: ypearson @. Sent: 11 April 2022 15:15 To: bernedom/SI @.> Cc: David Orchard (SXS UK) @.>; Author @.> Subject: Re: [bernedom/SI] Unitless types (Issue #106)

EXTERNAL EMAIL: Remember the 9 Phishing Indicators

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

— Reply to this email directly, view it on GitHubhttps://github.com/bernedom/SI/issues/106#issuecomment-1095106873, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYVEJ4C2U22TIQLEKQNJPJ3VEQXVVANCNFSM5TDHY33Q. You are receiving this because you authored the thread.Message ID: @.***>


Spirax-Sarco Engineering Plc. This e-mail has been scanned for viruses by Cisco Cloud Email Security.

ypearson commented 2 years ago

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 commented 2 years ago

Hi,

You are right that would be my preferred solution.

David

From: ypearson @. Sent: 11 April 2022 16:25 To: bernedom/SI @.> Cc: David Orchard (SXS UK) @.>; Author @.> Subject: Re: [bernedom/SI] Unitless types (Issue #106)

EXTERNAL EMAIL: Remember the 9 Phishing Indicators

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.

— Reply to this email directly, view it on GitHubhttps://github.com/bernedom/SI/issues/106#issuecomment-1095194110, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYVEJ4HWZ6LLAH5UBPDBMPTVEQ75RANCNFSM5TDHY33Q. You are receiving this because you authored the thread.Message ID: @.***>


Spirax-Sarco Engineering Plc. This e-mail has been scanned for viruses by Cisco Cloud Email Security.

bernedom commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

This will be in release 2.5.0

DavidTheLost commented 1 year ago

Hi,

If you make the constructor explicit you force the engineer to do:-

float data = 15.0F;
square_metre_t<float> c1 = 1_m;
square_metre_t<float> c1 = square_metre_t<float> { data };

This forces them to be explicit about the units and the following mistake becomes compile time error.

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

From: David Orchard (SXS UK) Sent: 11 April 2022 15:27 To: bernedom/SI @.***> Subject: RE: [bernedom/SI] Unitless types (Issue #106)

I see what you are getting at:

square_metre_t<float> c1 = 1;   // This is not square meters!!!

This does compile. You are saying 1 is a square meter. However, when the value is the result of a computation you are relying on the engineer not making a mistake and messing up his units.

square_metre_t< float > c2 = 1_m2;   // This is square meters!!!
square_metre_t< float > c3 = 1;      // This is not square meters!!!

using m2 = 1_m2;
float data = 15.0F;
square_metre_t< float > c3 = data * 1_m2;   // This is square meters!!!
square_metre_t< float > c4 = data * m2;     // Nicer!!!

From: ypearson @. Sent: 11 April 2022 15:15 To: bernedom/SI @*.**@*.>> Cc: David Orchard (SXS UK) @*.**@*.>>; Author @*.**@*.***>> Subject: Re: [bernedom/SI] Unitless types (Issue #106)

EXTERNAL EMAIL: Remember the 9 Phishing Indicators

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

— Reply to this email directly, view it on GitHubhttps://github.com/bernedom/SI/issues/106#issuecomment-1095106873, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYVEJ4C2U22TIQLEKQNJPJ3VEQXVVANCNFSM5TDHY33Q. You are receiving this because you authored the thread.Message ID: @.**@.>>


Spirax-Sarco Engineering Plc. This e-mail has been scanned for viruses by Cisco Cloud Email Security.