Simplify forEach, forEachRemaining, and removeIf conditionals

techsy730 commented 3 years ago

Also adjusts the conditional of Iterable.drv, Iterator.drv, and Collection.drv to be in line with the style guidelines in https://github.com/vigna/fastutil/issues/201

vigna commented 3 years ago

OK. I see how you managed the three-legged thing and it's probably better than how I'm doing it in Consumer/Predicate, so I might go in the same direction: a macro for the type of the argument of the right implementation method, and a conditional to choose between delegation by casting or by new lambda. In this way appropriate comments can be added.

Minor comments:

please refrain from using the <br> tag as it is deprecated as non-structural. A <p> is fine there;
as in Consumer/Producer, I would not deprecate the method taking a widened Consumer or Predicate because one might have a ready-made one. I would just add an @implNote (see Consumer) explaining the inefficiency.

vigna commented 3 years ago

I would also suggest adding tests for all types of overloads. I've done it for removeIf in shorts and ints. It is important not for the semantics, but to be sure that everything compiles as expected (e.g., no ambiguitity).

techsy730 commented 3 years ago

Ack on the tests; working on those now

techsy730 commented 3 years ago

No problem. I have a bad habit of being overly verbose to specify exactly what I mean, even when there is really no risk one would get the wrong meaning from a simpler statement. So helping me shorten my qualifiers is helpful. :)

vigna / fastutil

Simplify forEach, forEachRemaining, and removeIf conditionals #203