Here is the given reason for using a HashMap: https://github.com/SerenityOS/serenity/commit/f107c7065230c6...
Here is the given reason for using a HashMap: https://github.com/SerenityOS/serenity/commit/f107c7065230c6...
The code switches out a static array of size 3 for a hash table, in order to handle that incoming IDs (used as indexes in the array, then keys in the hash) can go up to 255 instead of just 2.
I would think that upping the array to size 256 would still be cheaper (or at least not a lot more expensive) in terms of memory used than adding a full hash table, but I have not inspected Serenity's hash implementation to validate that claim. It would almost certainly be cheaper in terms of performance, especially from less memory allocations.
That said, linearly scanning is not much different from a hash map with linear probing that has no empty slots left, so it's not necessarily obvious that the hash map actually takes that much more space.
I'd still just use a plain array, but my point was that hash maps aren't necessarily big and unwieldy.
Always start with an array if the problem allows for it, even if you need to reallocate sometimes, or have to scan linearly. Measure performance before using a map or linked list.
Even if not optimal, it definitely looks less unreasonable as a choice now that the original motivation found.
And if it had been “fixed” in the simple way without finding the original motivation, it very well may have introduced a different edge-case bug.
So one important lesson here, is always try to do some “due dilligence” to track down why code was written how it was before changing it, even if (or especially if) you assume it was just shoddy coding. (What’s the metaphor/aphorism used about this? I remember there is one, but can't recall/find it now).
(And also reminds us of the importance of good granular commits with good commit messages, to leave that history! Much easier to find original motivation than in a pre-version-controlled world).