On Wed, Feb 9, 2022 at 11:06 AM Gilles Sadowski <gillese...@gmail.com> wrote:
> Le mer. 9 févr. 2022 à 16:32, Gary Gregory <garydgreg...@gmail.com> a > écrit : > > > > Crafting a compressed file for a test fixture that causes a integer > > overflow deep in our code would be quite hard to reverse engineer. > > I of course agree that it would be overkill; hence the question, again: > Why > int size = ... > size = ExactMath.add(size, nameSize); > instead of > int size = ... > size = Math.toIntExact(Math.addExact(size, nameSize)); > ? > Uh... you tell me why the core you propose is better. > > The method in "ExactMath" considers that "nameSize" > must not be of type "long". If so, the fix does not belong > in that method. > Alternatively, it must be documented that the class is a > shortcut workaround for a specific issue (to be clarified) > and should not be mistaken as a general-purpose utility > to add two numbers (in which case "ExactMath" and > "add" are bad names for the intended purpose). > I've updated the Javadoc. Note that the class is reused in 13 call sites already to avoid pattern duplication. Also note that the class is documented as private, not that this will stop people from using it. Gary > > > > Gary > > > > On Wed, Feb 9, 2022, 09:38 Gilles Sadowski <gillese...@gmail.com> wrote: > > > > > Le mer. 9 févr. 2022 à 15:16, Gary Gregory <garydgreg...@gmail.com> a > > > écrit : > > > > > > > > Observe > > > > > > Perhaps I'm dense, but my detailed remark about the commit > > > reflects that I did. > > > > > > > that ExactMath delegates to Math after performing the necessary > > > > additional Math calls. > > > > > > As per the first part of the remark, it is not obvious why those > > > hoops are necessary; thus documenting the rationale would > > > prevent someone scrapping them (with just as terse a commit > > > message). > > > The other part of the remark signals a potential bug and/or > > > unintended behaviour; this also calls for clarification (and/or > > > unit tests). > > > > > > Thanks, > > > Gilles > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
