-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Avoid unecessary array allocations and initializations when performin… #1363
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
Conversation
maedhroz
left a comment
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.
LGTM, aside from some minor nits that you can take or leave
| } | ||
|
|
||
| /** | ||
| * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will |
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.
nit: It might be nice to use @link rather than code so things are navigable?
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.
The issue with link is that you can end up with a javadoc with a lot of link that made the generated javadoc harder to read. The general approach is to use simply use {@code } in the methods. I would prefer if we stick with the most common approach.
| * @param messageTemplate the template used to build the error message | ||
| * @param messageArgs the message arguments | ||
| * @param messageArg the message argument | ||
| * @throws InvalidRequestException if the specified expression is <code>false</code>. |
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.
nit: Might be easier to read quickly with a newline between the last @params and the first @throws
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.
I tried and felt that it was making the javadoc longer specially if I did the same when there was a @return. I checked the java classes javadoc and they do not use new line there so I decided to keep it this way.
We want to run the common tests in the subclasses of the abstract class, but not in the abstract class itself.
We want to run the common tests in the subclasses of the abstract class, but not in the abstract class itself.
…g query checks