Hello. Le jeu. 10 févr. 2022 à 20:40, Gary Gregory <garydgreg...@gmail.com> a écrit : > > 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.
It is better because it is self-documenting. > > > > 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. OK, this eventually makes it clear that your sense of "better" is tied to the number of spared characters... It still does not explain why the second argument cannot be a "long". If it's true, why not deprecate the method and define one with the appropriate signature? > Also note that the class is > documented as private, not that this will stop people from using it. IIUC, the class is unnecessary, but if it doesn't bother anyone else, the thread has been long enough. Regards, Gilles > 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
