-
Notifications
You must be signed in to change notification settings - Fork 103
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
Make static methods that ought to be static. Check if was intialized otherwise #35
base: master
Are you sure you want to change the base?
Conversation
5f69d18
to
5f2d97d
Compare
@rikba Could you have a look? |
Please 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution and exposing the functionality and throwing initialization errors. I think that's very valuable. SCOLGTM.
|
||
inline double rad2Deg(const double radians) | ||
{ | ||
return (radians / M_PI) * 180.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this a constant, i.e.,
const double kRad2Deg = 180.0 / M_PI;
Saves some devisions.
|
||
inline double deg2Rad(const double degrees) | ||
{ | ||
return (degrees / 180.0) * M_PI; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
Hi,
thanks for this nice utility.
I noticed that there are tow function that can be used outside the class GeodeticConverter.
I think it is more flexible to keep them static members or, even better, stand alone functions.
I also added exceptions when the other methods are used without initialization.
By personal opinion is that initialization is mandatory and that the constructor should force that, but I don't want to break the code of libraries which depend on this one.
Cheers
Davide