Convert to Kotlin

[veyndan]

Benefit A

Default values are still not supported in Moshi, yet Kotlin can support them natively. This means that List no longer needs to be nullable, with some obvious other examples being primitives.

Benefit B

Another benefit is that you access the Kotlin fields directly instead of having to use a getter.

Java

if (event.repo() != null) {
    event.repo().someFunction(); // Warning of calling method someFunction() on potentially null object
}

Kotlin

if (event.repo != null) {
     event.repo.someFunction(); // No warning occurs as nullability has been checked for
}

Benefit C

Java

event.repo().someFunction(); // Can cause NPE as event.repo() hasn't been checked for nullability

Kotlin

event.repo.someFunction(); // Would be a compiler error as event.repo hasn't been checked for nullability

Benefit D

Much more concise models in Kotlin, as builders, immutable objects, nullability status, are all extremely concise in Kotlin.

Benefit E

This isn't a huge benefit, but one less library (i.e. AutoValue) is used.

[Meisolsson]

Right now the moshi Kotlin support is built on reflection which from what I know is just as bad as in Java. There is work to bring generated adapters for Kotlin but in the meantime I'd prefer to stick with AutoValue for performance.

I've not tested the speed of reflection but I'm guessing that it's gonna be slower.

[veyndan]

I can't imagine reflection having an appreciable performance hit. I believe that the productivity improvement of using Kotlin will greatly outweigh the performance hit that reflection ensures.

[Meisolsson]

Yeah, you're probably right. Some thing to remember when doing this would be to use the Parcelable support in kotlin. So that we keep the code clean still.

[maniac103]

'Benefit A' makes it sound like items being nullable is a bad thing, but it - AFAICT - actually is not, as it allows distinguishing between 'item is not included in the response' and 'response contains empty item'. Same thing applies in the outgoing direction, where one definitely wants fields to be nullable as 'do not touch this field' marker.

[Meisolsson]

This depends. Some values like lists do benefit from not being returned as null as you don't have to check it for null but any operation on it can be the same. For other values we might still want null since we don't have a sane default value. So it all comes down to if there is a good default value or not.

[huperniketes]

Converting this SDK to Kotlin is a terrible idea. Dependencies will need to be updated, breaking existing projects, forcing developers to waste time yak shaving instead of improving their own projects for their users.

Additionally, Benefits A, B, C, and D are all symptoms of the same problem: null values in code, which is an even more terrible practice than reinventing the wheel and chasing the latest fad.

If the submitter wants a Kotlin implementation of the Github SDK, may I suggest he fork it, and create such an implementation. But those of us using the existing implementation would rather focus on our existing task backlog.

@maniac103, the reason null values are a bad thing is it conflates several possibilities rather than providing the client with insight into the result or state of the process. If we really want 'item is not included in the response' and 'response contains empty item' to be distinguished from other types of results, we should use different values to identify them. Instead of using null as a catch-all value, which is context-sensitive and requires more work to dig into the state and process originating it, the conscientious developer should make use of a unique class or object to signify the condition, intent or concept.