Hi guys, Which do u prefer and why? :one: ```publi...

# codereview

[George]

Hi guys, Which do u prefer and why?
1️⃣

public interface Payload {
    public val data: Any
    public val error: Error
}

with factory fun:

public fun <T : Any> Payload(data: T): Payload {
    return PayloadImpl(data)
}
public fun errorPayload(error: Error): Payload {
    return PayloadImpl(error = error)
}

2️⃣

data class Payload(val data: Any, val error: Error)

[George]

Fyi: It is to be used for all my public api respones.
IMHO: They are both equals.
For 1, you can extend it pretty easily if there is a need.
For 2, you can play with wrappers and converters if there is a need to extend it's properties/functionality.

My instincts tell my that the first option is not over engineering, since it seems more natural option for me to not base all my apis on a final class but rather on an interface/abstraction. (maybe im wrong ;P)

Im looking forward to hear your opinions. Thanks in advance for any answers!

[Roukanken]

well, a) the fields in Payload would need to be nullable

and b) if you have such different state, is this not a perfect job for sealed class / interface? In fact you can find near this exact concept in many variations: build in Result<T> , Arrow's Either<E, V> various other Result<V, E> in other libraries...

[George]

I agree on a. Tbh i had checked the build in Ressult type and is a value class, and that;s when i started doubting my approach with the interfaces/sealed. But on the other hand Either was a very good example to be fair. (Build in result type though is pretty awesome.)

[Joffrey]

it seems more natural option for me to not base all my apis on a final class but rather on an interface/abstraction

It's true to some extent, but I believe it applies more to behaviour than data.

I totally would prefer an option 3 with a sealed class, possibly even a generic one. Both examples (with the required nullable fields) will be weird to use in general compared to a sealed class.

[George]

Hmm, im was actually representing my "absent state" with an object

internal object EmptyData {
    override fun toString(): String = ""
}

And was checking for this object

public class PayloadImpl internal constructor(
    override val data: Any = EmptyData,
    override val error: Error = emptyError()
)

My empty error :

internal fun emptyError(): Error {
    return Error("", -1)
}

Even thought at first, it seems counter-intuitive to not represent the "absent state" with nulls, you get a more clean api without nullable data. But you still have to check for something like isErrorEmpty or isDataEmpty which is the corresponding logic for nulls.

Which would u prefer nulls or EmptyObjects ?

Note: i have a requirement for my object creation which requires that either data is empty or error but not both. It is pretty much the same for nulls or empty states. For example i have this:

/**
 * Requires that either data is empty or error but not both. Throws
 * [IllegalArgumentException] if the contract is violated
 */
protected fun checkState(data: Any, error: Error) {
    var flag = true
    if (data is EmptyData && error.isEmpty)
        flag = false

    if (data !is EmptyData && !error.isEmpty)
        flag = false

    require(flag) { "Either data or error must be empty, but not both. Got data: $data, error: $error" }
}

[Roukanken]

In what way is api cleaner with empty objects than with (Kotlin) nulls??

You are literally doing what Java is doing: inserting an near-invisible failure state you have to check for everytime and heavens forbid you forget about it.

[George]

Hmm, that's so true. Like Optional 😛

[Roukanken]

As for "one or other is empty": sealed classes would solve that perfectly AND you would get similar api as you have now, but it would be without empty objects.

Eg either you have a Data, in which case both you and compiler are sure that error does not exists (vice versa with error). Or you have a Payload (the parent class) and compiler will force you to first figure out if it's Data or Error

[George]

After some quick prototyping, yea i totally agree, sealed is the way to go

public sealed class SealedPayload
public data class Data<T>(val value: T) : SealedPayload()
public data class SealedError(val message: String, val code: Int) : SealedPayload()

It also plays nice with the generic, as joffrey comment out. Thanks guys for your feedback, appreciate it!

[Joffrey]

I suggest to have the SealedPayload be generic itself, otherwise you lose the type info when passing the parent type around, and this makes pattern matching in when less useful (you can't check for the generic type).

public sealed class Payload<out T> {
    public data class Data<out T>(val value: T): Payload<T>()
    public data class Err(val message: String, val code: Int): Payload<Nothing>()
}

(Also, note the use of out to tell the compiler that your class is covariant in T)

With the generic there, when some function receives Payload<T> and checks that it's a Data, it knows which T it is, and you can write type-safe things like:

fun <T> Payload<T>.valueOrThrow(): T = when(this) {
    is Payload.Data -> value
    is Payload.Err -> throw SomeException(message, code)
}

[George]

I was avoiding the generic on the Payload itself, because i did not think i can make the error be a type of Nothing .

Maybe it's an another question but how come i can write something like this and be ok

public fun test2(): SealedPayload<String> {
    return SealedError("interesting", 100)
}

With SealedError being a type of Nothing

But when i make the sealedError type of Any? i get compiler error type mismatch?.

[Joffrey]

Because the payload is covariant in T , which means Payload<Child> is a subtype of Payload<Parent>. Because Nothing is a subtype of everything, Payload<Nothing> is a subtype of any other Payload type

[George]

In case i need this struct to be serializable, i guess this changes the whole structure because nothing is not serializable?

[Joffrey]

Nothing is just a type parameter here. There is no value of this type at runtime anywhere here, or anywhere at all for that matter, because by definition it is a type for expressions that cannot resolve to any value, just to help the compiler in a way.

In which way do you expect it to affect serialization? If a payload is of type Payload.Data, then this is the class that will be serialized with a value of type T inside (depending on the serialization library of course). If it's Payload.Error it will be serialized with the code and message. Do you have anything specific that you think won't work?

[George]

Actually im using the kotlinx.serialization and im getting the error Serializer for element of type Nothing has not been found and i found the corresponding issue on the lib https://github.com/Kotlin/kotlinx.serialization/issues/614. But as you point out this is my desired behavior. Serialize data with value T or serialize Error with code and message.

[Joffrey]

Thanks for the link, I actually wasn't aware of this issue, TIL. In theory, there is nothing wrong here, so it should work. I guess that's just a bug/peculiarity of how kotlinx serialization looks for serializers. According to the conversations in the issue you should be able to write a serializer for Nothing and annotate the type parameter

[George]

Indeed, it works fine. I used the NothingSerializer from the link and specified the serializer for my file like this @file:UseSerializers(NothingSerializer::class) . Tbh i was just confused how i was supposed to serialize Nothing but the trick to it is to understand we never serialize Nothing .

[Joffrey]

Yep exactly, that's why I think it's wrong for kotlinx-serialization to fail due to a missing serializer for a type that is only used as type parameter but never serialized. This is actually the object of another issue:
https://github.com/Kotlin/kotlinx.serialization/issues/607

But even if they don't fix this one, providing a default serializer for Nothing is reasonable given that we know it's never going to be serialized