rock-core / base-types

C/C++ and Ruby definition of the base types in Rock
6 stars 39 forks source link

two new comparison operators to base/Angle #105

Closed romulogcerqueira closed 7 years ago

romulogcerqueira commented 7 years ago

add less than or equal (<=) and greater than or equal (>=)

romulogcerqueira commented 7 years ago

ping

jmachowinski commented 7 years ago

This looks a bit undefined. Can you describe, how this operator should work ? Keep in mind that there is the wrap around from -PI to + PI

romulogcerqueira commented 7 years ago

Hi @jmachowinski,

initially I created only two more operators, such as the existing ones (>, < and ==).

However, after your question, I remembered I can use the current operators with the logical negative operator to achieve the same results.

Then I am closing this PR.

doudou commented 7 years ago

This looks a bit undefined.

Given that the strict < and > operators are already defined, these look like fairly natural extensions.

Then I am closing this PR.

Don't. !(a > b) is a lot harder to read (ergo, makes the code much harder to maintain) than a <= b

romulogcerqueira commented 7 years ago

!(a > b) is a lot harder to read (ergo, makes the code much harder to maintain) than a <= b

@doudou, I agree with you. Should I reopen this PR?

jmachowinski commented 7 years ago

I agree, as the < > was already defined, this is a natural extension. Sorry, didn't see this. Anyway, we should perhaps document the behaviour of these operations. It looks like we normalize to -M_PI <-> M_PI. This then results in 181 degrees < 179 degrees.

romulogcerqueira commented 7 years ago

It looks like we normalize to -M_PI <-> M_PI.

Actually the canonization is in -M_PI + epsilon <-> M_PI.

This then results in 181 degrees < 179 degrees.

Correct.

doudou commented 7 years ago

This then results in 181 degrees < 179 degrees.

Not really, since Angle cannot represent 181 degrees by definition ... but I'm always for more documentation.