Closed william-reed closed 10 years ago
Awesome. Thanks :D
The only thing I can pick out is that ...in class#methodName
is redundant, seeing as the stacktrace should provide that information anyway.
That is true but with any overloaded methods the messages provided help to quicker distinguish between each of the overloaded methods resulting in faster debug time. Of course if you still would like me to get rid of that I will do so. On Aug 18, 2014 7:42 AM, "DSH105" notifications@github.com wrote:
Awesome. Thanks :D
The only thing I can pick out is that ...in class#methodName is redundant, seeing as the stacktrace should provide that information anyway.
— Reply to this email directly or view it on GitHub https://github.com/DSH105/HoloAPI/pull/108#issuecomment-52481373.
I'd rather see you using Preconditions, since we're using that everywhere else. (Or should be using everywhere else)
Also, there must be one space between curly-bracket, and the "normal" brackets.
@CaptainBern Before I had started this @DSH105 had told me to do it this way so I am not sure how he wants it. I fixed up the formatting issue.
@CaptainBern I only saw the use of Preconditions in one other class and from my research Apache Commons has a smaller impact than Guava preconditions in Google Common
Thank you!
The Issue This PR is here after askign DSH105 how I could help the API and this was one thing suggested so here it is.
Justification for this PR: This PR will help programmers when debugging there code as the new notNull checks will speed up developers determining reasons for NullPointerExceptions.
PR Breakdown Basically all that is added is Validate.notNull checks on the classes that implement Hologram. Some other code was also added to check to make sure that any index was not < 0.