On Wed, 17 May 2023 23:17:30 GMT, Andy Goryachev <[email protected]> wrote:
>> This is also solved in #1076: >> >> if (obj instanceof BitSet<?> bitSet && >> getElementType().equals(bitSet.getElementType())) >> >> But since that requires pulling in even more changes (`getElementType`) I've >> left it there. >> >> Relying on subclasses being final though seems like a bad idea for an >> abstract class, unless we seal it and use permits (or, just remove `BitSet` >> completely... it's purpose will be minimal if #1076 is accepted). > > that's my point. I suppose we could use sealed class here, since javafx > requires minimum java17? Do you think this needs to be changed as part of this PR? Note that the `==` solution requires: - sealing BitSet permitting only PseudoClassState and StyleClassSet - adding a warning that if you ever unseal it, that you must re-evaluate the `equals` implementation - adding a warning to PseudoClassState / StyleClassSet to re-evaluate `BitSet#equals` if you remove `final` The `instanceof` solution allows arbitary subclasses, like any other Set implementation does (`AbstractSet#equals` for example use `instanceof Set`) and requires no further changes. What I'm missing is the reasoning why you would want to change this. I realize that the other solution can also work, but that's not enough IMHO. For me the two options are practically equivalent, neither being truly better than the other, so then it is a matter of preference, and I slightly favor using `instanceof` here to avoid potential problems down the road. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1070#discussion_r1197182269
