-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Lang 1134 #87
Lang 1134 #87
Conversation
Very nice, please see my comments. |
* @since 3.4 | ||
*/ | ||
public static void notNaN(final double value, final String message, final Object... values) { | ||
if (value != value) { |
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.
Better use Double.isNaN(double)
here. Reads better, IMHO.
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.
Okay, I wanted to avoid having more method calls as the Validate methods might get called often in a program.
Should I edit it and make a new pull request? How does it work?
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.
Hello @Lady-Stardust,
calling a method is not that expensive (approximately 2ns), so we can live with that ;-)
You can simply add more commits to your branch and github will update the PR when you push your changes to your fork. Thank you!
Ok I pushed the changes to my fork. I made the changes you said, except for removing the "Obj" suffix because it wouldn't compile anymore. |
Very nice, I'll have a look later this week! |
@Lady-Stardust, what is the name you want to be listed with in changes.xml? |
I don't want to be listed |
…berUtils.isNumber() This closes github #87 thanks to pbrose
LANG-1134