kevinrr888 commented on PR #4852: URL: https://github.com/apache/accumulo/pull/4852#issuecomment-2331959854
> > Not sure if there was a specific reason for using null before; in the case of MetadataConstraints it appeared unnecessary. The other implementations of Constraint still return null for check(). > > @kevinrr888 I think the reason behind the previous null code was that normally violations are not seen. So for the normal case the code was attempting to avoid allocating an arraylist that would never be used. Not advocating for carrying that behavior forward, just explaining the rational behind that code. I see. A potential issue with this new impl is now this one impl of `check()` is different from others in that it doesn't return null. This is documented, but may be confusing. Wondering if it would be better if the other impls were changed to no longer return null or this impl should somehow be changed so null is still returned. Or could be perfectly fine as is: just being an exception to the other `check()`s -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
