Hi everybody! Which option would be the most idiom...

# codingconventions

[Liudvikas Sablauskas]

Hi everybody! Which option would be the most idiomatic and most readable to you personally?

fun getConfiguration(softwareInformation: SoftwareInformation?, newVersion: Int): Configuration? {
    // 1
    return when (softwareInformation?.supportsUpdate) {
        true -> null
        else -> Configuration(version = newVersion)
    }

    // 2
    return if (softwareInformation?.supportsUpdate != true) {
        ControllerSoftwareConfiguration(version = newVersion)
    } else {
        null
    }

    // 3
    return softwareInformation?.supportsUpdate?.takeIf { !it }
        ?.let { ControllerSoftwareConfiguration(version = newVersion) }

    // 4 Some other approach?
}

[Rohde Fischer]

either use an Either monad such as from ArrowKT. Or use a sealed class/interface

null is still (with the improved null-handling) a nasty solution. Even if one is truly able to prevent all the old null issues (I sincerely doubt it) it still has the weakness of having an implicit meaning that will never truly be consistent (no matter how stubbornly people will claim otherwise)

using either Either or sealed allows you to explicitly and clearly model what is going on, so you will end up returning:

true -> NoSoftwareInformation.left()
else -> Configuration(version = newVersion).right()

or

true -> Configuraion.NoInformtion()
else -> Configuration.WithInformation(version = newVersion)

that said, as a side comment, your function should probably never be called in the way you designed it

rather than do:

val softwareInformation = getInformation()
foo.getConfiguration(softwareInformation, newVersion)

do:

val softwareInformation = getInformation()
softwareInformation?.let { foo.getConfiguration(it, newVersion) }

that way you do not need to design the ugly and bad null handling. Also this will probably lead to a design raising the question of why softwareInformation is even nullable in the first place - but of course that depends on your project etc.

[Liudvikas Sablauskas]

Thanks for the input.
I don't think it makes sense to introduce Arrow into an already big project, it feels like something which should be introduced from the start, since it's such a core architectural approach.
And the issue with Either or Option in this case is that it seems like you are still handling null , but you're just making it explicit the form of a class. In the end it's the same, null just has the benefit of convenient syntax sugar. So using Either doesn't seem like a big win here.

I guess I don't see a big difference between having a Configuration? and a Configuration which is a sealed class with some None subclass. The second option seems even worse to me, more verbose definitely.

I do agree that restructuring would help which would remove the need for null in the first place.

[Rob Elliot]

Personally I'd be inclined to go with:

val supportsUpdate = softwareInformation?.supportsUpdate ?: false
return if (supportsUpdate) null else ControllerSoftwareConfiguration(version = newVersion)

Makes it explicit that the default value for supportsUpdate is false and then it's a simple ternary with no double negatives.

I'd be wondering about refactoring getConfiguration to make softwareInformation non-nullable though... it feels smelly. Could you have a default instance of SoftwareInformation where supportsUpdate is false?

val noInformation = SoftwareInformation(supportsUpdate = false)
val configuration = getConfiguration(softwareInformation ?: noInformation)

[Rohde Fischer]

I probably wasn’t too clear, I actually avoided the Option -class, basically I see it as a slightly prettier version of doing `null`s (which imo is a bad design)

tbh. I’ve never understood why Either (and a few others) aren’t part of the core libraries, but that’s a different talk

the major difference between the None and what I propose is being explicit in the naming - which apart from being more useful also allows to communicate various reasons for a missing object (i.e. Configuration in this case), which allows you to communicate your domain (i.e. rules and understanding) rather than null or None both of which doesn’t really say much

[Joffrey]

If there is no more semantics to communicate than "this is not there", null is perfectly fine IMO, but I get your point. The thing about null is that it benefits from some syntax sugar that makes the code really neat to read. Optional is just a disguised null that doesn't have the syntax sugar, so it's usually a bad idea to use it on Kotlin. Either is not part and will probably not be part of Kotlin stdlib because it's part of a different programming style (railway programming) than the idiomatic Kotlin. The closest thing we're likely to get is union types, or some way to make custom sealed classes more practical to use.

[Rohde Fischer]

I guess this reaches the slightly softer ground, but yeah as you probably guessed I’d still model it. Multiple reasons, most of which boils down to the same reasons as why binary enums should not be modelled as a boolean 😉 (A simple way is to ask how would you refactor the second it goes from “not there” to multiple interpretations? Spending a little time modelling will make this and other refactorings easier)

from the ones that go beyond that is that there are still a lot of cases where a `null`able value accidentally ends up being null because of an error, violating the logic of “this is not there”. The second there’s almost any framework utilising any kind of reflection to populate values the risk is there (ORMs and deserialisers as the most common). Where it is true that Kotlin’s design guards against a lot of the null-issues it does not mean that Kotlin is immune

To me null is still a bad design (but necessary) decision and should only be used long enough to unwrap from systems that utilise it (legacy code, external frameworks etc.). Just because it got a lot better and less error prone in Kotlin does not to me mean that we should embrace it, I will need more/other arguments than that - especially since I see the other design as better, even if null were perfectly safe

Kotlin is strongly functional, so I see Either as a perfect valid and valuable addition. It is also the best way to model in any DDD setting, unless you want to specify all domain errors as something sealed. I think Either (whether it’s called Either , Result or something else) is the most sensible way to get away from the malpractice of using exceptions for domain errors. Exceptions should be for exceptional cases and only programming related errors. Cases such as someone trying something they are not allowed is a business/domain rule and should be modelled appropriately 😉 There are probably also other ways to model that, but from what I’ve seen so far, I see sealed classes/interfaces and Either as the viable solutions (feel free to teach me a better way if there is, I’d be very happy if I’m wrong)

[Rohde Fischer]

as a side comment, since the original question was about the idiomatic way, it is worth commenting that what I am suggesting is not (at least currently) idiomatic, but rather actually questioning the current idiomatic way

[Liudvikas Sablauskas]

Thanks for the valuable discussion. I do agree that you should always strive to get rid of null as early as possible. I feel like I often run into issues where this doesn't seem reasonable for some reason. Two common cases come into mind:
1. A model that has a lot of fields which are all nullable for their own separate reasons. There are a lot of good examples for this. For example, some kind of Form that has a lot optional fields. Or a User class, which might have some key information and a lot of optional stuff, which is not provided, like:

data class User(
    val name: String,
    val email: String,
    val address: String?,
    val phone: String?,
    val gender: Gender?,
    val petInfo: PetInformation?,
    val relationshipInfo: RelationshipInformation?,
    // etc.
)

In this case, would you argue that Gender, PetInformation and RelationshipInformation should all be sealed classes and have their own NoInformation instead of being nullable? Perhaps so, but I see 2 drawbacks. #1: a field being nullable already has an immediate clarity that screams "no information available" (but I do see your argument that just null doesn't have the namespace and when passing it around it becomes unclear which class the null empty state originally came from, which the sealed class could help with). #2: The NoInformation needs to be consistent throughout the code. Otherwise, if you call it NoInformation here, Empty in another place, NotGiven in another one, then you cause more confusion than with a null, since all of those things represent the same situation: the value is not given.
And even if you did that, it still leaves address and phone, which are basic Strings. So basically, in this case, would you say that every single nullable field should be a sealed class with it's own NoInfomation ? Seems a bit.. overkill 😕
Another sad caveat is that if I check somewhere that if(user.petInfo != null) { doSomething(user) } , inside doSomething the compiler still doesn't know that petInfo isn't null, so I'm forced to check again. In this case, having sealed classes wouldn't even help. I thought contracts could fix this, but no cigar. But I guess this is a separate issue.

2. Another example of kinda forced nullability is a large model which has nullable fields that get filled up during runtime. Since the model is big, making new data class with just those fields taken out seems kind of overkill. Once again you are forced into nullability. This was the case in my situation. softwareInformation is part of a parent data class and it's filled some time later. (Not via var but via copy of course). And multiple fields in the parent data class do that, they are nullable and later get filled.

[Joffrey]

I believe the use case of filling up something later is a bad example for the need of nullability. IMO this is actually better served by defining different types having or not this property. Before filling in this data, you know for sure even at compile time that all those things are absent, and after filling these in, you know for sure that they are there. It's not like you need to dynamically decide anywhere whether the value is null or not at runtime.

[Rohde Fischer]

@Liudvikas Sablauskas yes, I believe that NoInformation is still better (although only marginally). Better is NotProvidedByUser or similar, so it’s entirely clear what the reason is (which is part of the point 😉 ) That said, I’d also be hesitant about how to represent forms with that kind of optional logic, unless it’s made explicit in the form - I’ve seen so many errors on stuff like a form is null or undefined until the user touched it, but then it’s “touched” by no data is filled in, what is the truth here? I think especially the form case actually provides a prime example on why null makes things worse. A text field for instance contains a string (that might be empty) and that’s it. The null here is an example of some developers trying to be “clever” and putting multiple states into something that should’ve been a separate state - and that actually also invalidates your own statement, because the null does not convey NoInformation, it actually conveys more (which it shouldn’t and which would’ve probably been clear if that nasty pattern of using `null`s in forms had never been established

also I will question the logic of NoInformation having to be consistent throughout the code more than most other cases. To begin with NoInformation is a bad name, because it conveys exactly the same information as null and Option.None, and in the case provided all of them actually masks the actual situation causing misleading situations. Think of it in terms of the domain, what is happening? Did the user touch the form/field? Did the user decide not to fill the form/field etc.? Convey that information, even if it’s unwrapped to a simpler default value later (but if so, keep in mind that you get information loss at that point)

The typing example could be “hacked” using generics and sealed classes, but that said that solution is very forced, and I would almost certainly consider it wrong 😉 That said, I also think the case is so rare that it’s fine, and in most cases I’d also inclined to say that doSomething should do it’s own consistency checking. Basically the type checking in this case would - even for simple cases - become hellish, and I wouldn’t want to go out through that path, unless a UserWithPet-model is relevant (it very well could be, and probably is way more than people model it, IMO it seems people do not model no where enough)

In the large model case, I guess there are two cases. In the one where I’m in control of my models, I’d remodel. If I got a model (and/or model hierarchy) large enough for this to be an issue, I’m almost certainly doing something wrong (there might be exceptions, but I have yet to see them), models large enough for that sounds like a “God model” that does way too much. The second case is where the model is outside my control - my answer to that is probably predictable 😉 basically split the model into smaller units, so it makes sense - and also if possible filter out anything not needed

That also in almost all I’ve done solve the mutability issue, one exception is my work with DNA analysis, where using copy wasn’t viable - at least not if it were to run on a sane machine. I cannot remember what we actually did back then, but today I’d wrap it in a local mutable model during the calculation that is only accessible in a tightly controlled environment (ofc. be aware of the concurrency implications in that case) do the mutable work and then output the result in an immutable class once outputting it.

One note on the last part is that I usually don’t do UI, but I am aware that the mutability part has it’s own implications there, since the speed of user events etc. also (at least in some cases) makes immutability unviable - however, if I’m not mistaken the same pattern as above (local mutable model) should be applicable - I am also aware that a lot of frameworks aren’t designed that way, so it might be us as application developers that need to shield the mess (just as we should shield against the `null`s from forms/fields)