On Tue, 21 Apr 2026 12:45:12 GMT, jengebr <[email protected]> wrote: >> test/jdk/java/util/Collection/MOAT.java line 1495: >> >>> 1493: m.clear(); >>> 1494: m.putAll(source); >>> 1495: check(m.equals(source)); >> >> Hm. It turns out that `putAll` is barely tested in MOAT (only a couple >> special cases) and so the test assertions made here should be very general. >> Maybe something like this: >> >> int oldSize = m.size(); >> Map<Integer,Integer> source = Map.of(10, 1000, 11, 1001, 12, >> 1002); >> m.putAll(source); >> check(m.entrySet().containsAll(source.entrySet())); >> check(m.size() == oldSize + source.size()); >> >> This seems to be the only place in this file where `putAll` is called on a >> non-empty map, so maybe preserve that and adjust the assertions to match. >> >> It's kind of a shortcut to use the entryset for this, and in principle >> perhaps one ought to use `containsKey` and `get` for testing individual keys >> and values. But if `putAll` is broken, the map's entryset is probably also >> broken and this will catch it. >> >> The source map can be of any type, not HashMap, so might as well use >> `Map.of()` to create the source. > > Thx for the examples, I detect an emphasis on code density that I'll apply to > future contributions. Map.of() isn't my favorite, for various reasons, but > it makes perfect sense here. Appreciate the explanations on these, too!
OK, thanks. I guess though, I'd like not to over-emphasize code density. Certainly not to the point of "code golfing" to minimize the number of characters or anything like that. In the HashMap.java code the density of code is driven by reduction of the effective scope of a local variable and to avoid unnecessary field loads into local variables (as is the prevailing style in that file, from Doug Lea and Martin Buchholz). In the benchmark, if there's duplicate code, then yeah it ought to be removed. But there are other times I'll favor clarity over brevity. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28243#discussion_r3135100586
