iamaleksey commented on code in PR #4119: URL: https://github.com/apache/cassandra/pull/4119#discussion_r2082517383
########## src/java/org/apache/cassandra/replication/Offsets.java: ########## Review Comment: I don't mind it too strongly - just saying that in all the cases here where `Immutable` is introduced in this PR, I don't see any risk if `Mutable` were used instead. The line between defensive programming and paranoid programming is a fine one :) We have a ton of `ArrayList`, `HashMap`, `HashSet` objects in all kinds of transient objects, requests and responses included, but in all the years working on C* I'm yet to see it cause a bug in any code I wrote, reviewed, or fixed ¯\_(ツ)_/¯ And it does come with a price of having extra code to maintain, more code to fit in the cache, and often turning nice monomorphic inline-able call sites into less efficient ones. But I won't fight you on this, for I don't feel strongly enough in this case. I did review these changes for correctness, they seem fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org
