https://kotlinlang.org
Join Slack
I noticed that two subtypes of a sealed class can be added to a Set with the same value. This is pro...
# announcements
f
I noticed that two subtypes of a sealed class can be added to a Set with the same value. This is probably because Equals across different types will not return true. What would be the recommended approach to prevent same value, but different type of the same sealed class in a Set?
n
If C1 and C2 both inherit from P, then you just need to make sure that you are working with Set<C1> or Set<C2> as appropriate.
And not working with Set<P>, which is you saying that you're entirely ok with having both C1 and C2 in that Set
You need to be careful because kotlin will have a tendency to decay to the parent type in certain situations
f
its the second case, Set<P>, and I understand why this is the behavior but I actually want to achieve the opposite.
n
You want a Set<P> that only contains C1's?
f
I want a Set<P> that contains C1 or C2, but only one of them with the same values
if I already have C1, with value 1, then I dont want C2 with value 1
m
You could add the equals and hashcode functions to P and make them final. Then you need what every equals logic you are intending. Just make sure you are honoring all the contract requirements for equals.
n
Yeah, I was going to say, then you'll need to redefine equality
Kotlin Set doesn't support setting this independently of the type, so it does mean that == will now have that meaning for your P whenever it is used
m
And does that meaning of equality make sense outside of what you are doing with the set.
n
Some languages provide standard containers where you can actually control the way hash/equality works separately of the type that's passed in, which would possibly be useful here
it is possible that refactoring would be sensible though, perhaps? E.g. perhaps make P/C1/C2 and enum, and then you could have Map<Int, P> in this example
now, for any given set of state (the integer), it's either not contained, associated with C1, or associated with C2, which seems like how you wanted the Set to work
m
Yeah, using a map where the value is the key would likely create the desired effect.
e
TreeSet does let you use your own comparator, so if you have total ordering that is an option
f
the purpose is to make state explicit with the type
I have 
sealed T { data class AvailableT() data class UnavailableT() }
and in the same Set not have AvailableT and UnavailableT with the same values at the same time
maybe that is stupid 😅
e
@Nir's suggestion (as I understand it):
Copy code
sealed class P {
    abstract val x: Int
    class C1(override val x: Int) : P()
    class C2(override val x: Int) : P()
    override fun hashCode() = x
    override fun equals(other: Any?) = x == (other as? P)?.x
}
setOf(P.C1(1), P.C2(2)) + P.C2(1))
my suggestion:
Copy code
sealed class P {
    abstract val x: Int
    class C1(override val x: Int) : P()
    class C2(override val x: Int) : P()
 }
sortedSetOf(compareBy(P::x), P.C1(1), P.C2(2)) + P.C2(1)
either way the resulting set is [C1(1), C2(2)]; the additional C2(1) is determined to be "equal" to C1(1) and not re-inserted
I prefer the latter (if you don't mind TreeSet's binary behavior instead of HashSet) because having object equality work like that feels unexpected
otherwise... yeah you should probably use a Map with the extract unique keys and your values in the values
f
yeah I can see the point of being unexpected
map would be a bit pointless because then the objects would have no purpose of holding a value
if the value is moved to the map key
e
well it might still have a purpose if you're passing them off to some other piece of code, and it's not like embedding the key into the value costs much (just an extra reference)
f
the sortedSetOf is just to be able to use a comparator?
e
compareBy(P::x)
is a 
Comparator<P>
there, yes
f
i meant the choice of sortedSetOf instead of setOf
not that it is a problem but it changes the data representation
would be nice to have the same option with a regular set
e
oh yes, setOf() will default to being backed by a HashMap which uses hashCode(), which means you can't define a local version of equality
f
hmm
e
but both TreeSet/SortedSet and HashSet are Set
f
yeah I know
maybe one last question
what about having
Copy code
sealed class P(open val x: Int){ 
    data class C1( override val x:Int) : P(x) 
    ...
}
so the x must be defined in all subtypes?
e
if you define it like that, then there's two backing fields: one in P, and one in C1 (which also has the one in P)
not the end of the world but I don't like it due to the possibility of introducing inconsistencies (e.g. if you added
Copy code
class C0 : P(0) {
    private val rng = Random()
    override val x: Int
        get() = rng.nextInt()
then the field and the property no longer have any correlation to each other)
or in a less unusual example,
Copy code
class CMut(override var x: Int) : P(x)
I'd rather just leave 
abstract val P.x
with no backing field
either way, yes, 
x
must be defined for all subtypes
you could design a comparator which does not require that
f
hm
I wrote a test for it and the override method only works if I declare in both subtypes
ah but I'm also doing a direct equals comparison
without the Set
that way really only if defined in both types
nevermind 😂
n
@Fábio Carneiro the thing is that from the sounds of it the state is identical in both cases
in order to have equality defiend between C1 and C2, there must be a common set of state, call it S, that is being compared
unless you have some really fancy comparison logic that is doing some kind of "translation" but that's getting further down the hole
f
it didnt work on the Set either
so I moved the overrides to the subtypes
and then it works
n
So, I'd probably suggest something like
Copy code
class foo(val s: S, val p: P)
i.e. encapsulation over inheritance
f
I think when the comparison happens on the Set, is also not considering overrides on P level
n
foo holds the common state
P is a base for C1 and C2, and can either be an enum, or hold whatever state is not common between them
f
yeah perhaps an enum to represent the state also works
n
Yeah, if all the state between C1 and C2 is the same, it's better to factor it out, without any inheritance involved, and then just have C1 and C2 as enums, IMHO
f
but anyway the equality comparison would have to be modified, to ignore the enum property
n
with 
foo
as above, you can either have 
Map<S, P>
, and now each entry implicitly forms a foo, or you can have 
Map<S, foo>
, just a matter of convenience
yes, it would
Actually, not really, in this example
f
but the idea was really to make it explicit on the type
to keep it implicit then no need to hassle
n
If you have say a Map<S, foo>, it's keyed just on S, and S's default equality/hash are fine
what exactly do you want to make explicit?
f
AvailableT, UnavailableT
n
You want C1 to have an integer field called 
x
and C2 to have an integer field called 
y
, but you want to try x == y as equality?
f
clear types
so I can typehint on AvailableT, when I dont accept an unavailableT
n
what's weird about this example is that AvailableT and UnavailableT happen to hold the exact same state, and be equality comparable
even though they are completely different types
if their state is the same by "coincidence", then comparing them at all and designing based on that is fragile. If their state is the same by conception, then the code should reflect it.
f
its they represent the same concept in different states
its actually an entity
n
Then I guess I'd probably go back to something like 
open class foo(val s: S)
f
the comparison is by identity
n
and now C1, C2 both inherit from foo
so, they both get the same 
S
state
and you can accept things by C1 or C2, and these will include the 
S
which seems like what you want
actually, sorry
no, wait, that's fine 🙂 And now you still just have, 
Map<S, foo>
so, you don't need to override == or hash for 
foo
, or anything else
should be 
sealed class foo(val s: S)
I guess
f
do you mean 
open class foo(val state: S, val identifier: I)
?
e
Copy code
sealed class State {
    object Available : State()
    object Unavailable : State()
}
data class Entity<out S : State, out Key>(state: State, key: Key)
typealias AvailableT = Entity<State.Available, Int>
typealias UnavailableT = Entity<State.Unavailable, Int>
sortedSetOf(compareBy(Entity<State, Int>::key))
n
@Fábio Carneiro what's identifier? No, not really
f
Oo
n
more like:
Copy code
sealed class State(val s: S) {
    class Available(s: S) : State(s)
    class Unavailable(s: S) : State(s)
}
very simple
e
depends on how isomorphic the representations should be
f
but this is what I had in the beginning
because the subtypes are different and the comparison is between them, the result is false
n
No
That's if you use a Set
I said from the get go, use a Map
Map<S, State>
and it all just works
f
yeah but then it does not make it explicit
its fine, but its a different approach
or does not achieve what I had in mind
valid compromise
n
I don't really understand what you mean by explicit anymore. It's explicit in the sense that other functions can say that they want a parameter 
a: Available
e
a set with partial row equality and a map are the exact same thing
f
not really
n
What's the difference between the Set you wanted, and this Map?
can you show me the exact operation which is now less explicit?
f
in a map I can do
Copy code
mapOf<Int, P>(
    1 to C1(1),
    2 to C1(1)
)
I can put a key that does not match the value
n
I see what you mean
Yes, this is a consequence of the same state being present twice
f
yes
and not using the encapsulation
n
What you could do quite easily is wrap the Map with a class that meets the 
Set<State>
API
But basically this is why I preferred the non-inheritance approach
with inheritance, you make it hard to get at the pieces of state individually, that duplication is forced on you
f
I'm a big fan of composition, but in this case It would force me to decouple the state as its own concept
n
originally, you simply said that you wanted to be able to define functions that require accepting an Available or Unavailable, right
f
its completely valid approach and many softwares work like that
n
wasn't that your main objection?
f
and at the same time have a group of non repeated objects
compared by value
but ignoring the state
I was expecting the sealed class to work like that
but I get why it doesnt
this would work perfectly fine (without any override) if the requirement of the Set wasn't there
all the tests are passing now (with the override duplicated in the subtypes), but its ugly 😄
n
Copy code
sealed class P(val s: S) {
    class Available(s: S) : P(s)
    class Unavailable(s: S) : P(s)
}
class foo private constructor(val s: S, val p: P)

fun makeAvailableFoo(s: S): foo ...
fun makeUnavailableFoo(s: S): foo ...
okay
so, I think if you now override == and hash for 
foo
to ignore 
p
, that might be ok?
f
yes, that would probably mean only one override
but then I cant typehint on AvailableFoo, Unavailable Foo (only on Foo)
n
You typehint on Available and Unavailable
f
would have to do what @ephemient said above, with the Typealias
cant do that because what I pass around is Foo
n
yes...
Copy code
when (x = foo.p) {
    is Available -> some_func(x)
and so on
f
everywhere?
this misses the point
n
what do you mean, you would still need to downcast or use when?
No....
You have a Foo, you still need to cast it down in order to call a function that takes FooAvailable
f
because if I have 
fun(foo: AvailableFoo)
I dont care about UnavailableFoo
and now I have to put checks everywhere
n
you would need to put checks everywhere anyway
(foo as AvailableFoo)?.someFunc()
f
but this is the difference with the inheritance
Foo doesnt exist
n
huh?
If you only define functions on derived members of an ineritance hierarchy
f
with the sealed class it can either be one or another
n
you won't be able to call them using a base class
you need to somehow downcast
the only way to avoid downcasting is to define the function on the base
f
I can typehint on the sealed type or any of the subtypes
n
yes, you can do that here too
it's exactly the same situation
f
but I never have an instance of the sealed type
and with this approach it happens
I have an instance of Foo which I need to get rid off
n
not sure what you mean. I can assure you that this approach is letting you do exactly the same thing as straight up inheritance
the inheritance is in a member variable, that's the only difference; it's the same
when i suggested this the first time, the issue was that C1/C2 didn't have access to S, which indeed made certain things awkward
f
Copy code
sealed class P {
    abstract val x: Int
    class C1(override val x: Int) : P()
    class C2(override val x: Int) : P()
    override fun hashCode() = x
    override fun equals(other: Any?) = x == (other as? P)?.x
}
here I can't do 
P(1)
n
that's why I changed it so that C1/C2 do have access to S
f
I can only do 
C1(1)
or 
C2(1)
n
yes, and in m y example you cannot construct Foo's directly
you can only call 
makeAvailable(1)
or 
makeUnavailable(2)
rename those functions to C1 and C2
f
ah
n
well, guess you can't literally do it for clashing reasons, but it's exactly the same thing
f
I didnt see the private const
n
yes, it has to be private so the state is kept in sync between the top level Foo, and the derived stuff
it's a bit ugly and it wastes a little storage, but it's probably the closest to being able to capture all the things you want
f
still requires me to put 
when
everywhere
I wont be able to typehint on the state type
only on Foo
which can be either
fun whatever(foo: Foo) {}
vs 
fun whatever(foo: AvailableFoo) {}
the first I need to consider the state, the second I dont
n
🤷 there's the same amount of type hinting as with regular inheritance
In your original approach, if you have a 
P
, you cannot call a function that accepts a C1
f
yes but not typehint that considers state
but in my approach P doesnt exist. you cant have an instance of it
you cant have an instance of a sealed class
n
huh
the actual type of the object may always be C1, or C2, but you can have objects, for which, you only know the type to be P
and when you have such an object, you will not be able to call a function that takes a C1
you'll need to use when or smart cast
you can certain have 
fun someFunc(f: Foo)
in your approach, even if Foo is a sealed class
and inside the body of 
someFunc
, if you have 
fun someOtherFunc(c1: C1)
, you can only call 
someOtherFunc
from 
someFunc
if you do some kind of cast
honestly, this stuff is exactly the same between the two approaches
f
you can certain have 
fun someFunc(f: Foo)
 in your approach
you can but you dont have to
you can use Foo if any works, AvailableFoo if only available works
n
you don't have to in my approach either
f
and you skip the When
n
yep, same in mine
In my code, you could already have an instance of Available
and then you don't need the when
f
but I dont need a instance of a empty value state object
it doesnt mean anything
n
what's the empty object in my example?
f
it only means something when combined with the value/identity
ah right I see what you mean
I forgot you had the property on Available
n
I think you didn't read the 10 lines of code I showed very carefully tbh
f
I did, but they moved away
n
anyhow, it really is the same from a type hinting perspective, other than having to write 
f.p
instead of 
f
, that's really the only difference, and it saves you having to write multiple overrides, and generally cleans up the way that hash == works
so, I think this is probably the best you can do
gotta go, good luck
f
Ill give it a try replacing with my approach and see
thanks a lot
👍 1
4 Views
Open in Slack
PreviousNext
Powered by

kotlinlang

A modern programming language that makes developers happier.

Powered by