Java and Kotlin antipattern evolution.md

Singleton

1. Roll-your-own lazy singleton

   public final class Single {
       private static Single INSTANCE;
       private Single() {}
       
       public static Single getInstance() {
           return INSTANCE != null
                   ? INSTANCE : (INSTANCE = new Single());
       }
   }

2. Roll-your-own DCL singleton

   public final class Single {
       private static final Object LOCK = new Object();
       private static volatile Single INSTANCE;
       private Single() {}
       
       public static Single getInstance() {
           Single inst = INSTANCE;
           if (inst != null) return inst;
           synchronized(LOCK) {
               if ((inst = INSTANCE) != null) return inst;
               return INSTANCE = new Single();
           }
       }
   }

3. Enum as singleton

   public enum Singleton { INSTANCE; }

   Healthy person's singleton:

   public final class Single {
       private static final Single INSTANCE = new Single();
       private Single() {}
   
       public static Single getInstance() {
           return INSTANCE;
       }
   }

   object Single

4. A singleton with a parameter:

   public final class Single {
       private static Single INSTANCE;
       public static Single getInstance(Context context) {
           return INSTANCE != null
                   ? INSTANCE : (INSTANCE = new Single(context));
       }
       private final Context context;
       private Single(Context context) {
           this.context = context;
       }
   }

   class Single
   private constructor(private val context: Context) {
       companion object {
           private var INSTANCE: Single? = null
           fun getInstance(context: Context): Single {
               INSTANCE?.let { return it }
               return Single(context).also { INSTANCE = it }
           }
       }
   }

   Healthy person's class with a parameter:

   public final class NotSingle {
       private final Context context;
       public NotSingle(Context context) {
           this.context = context;
       }
   }

   class NotSingle(private val context: Context)

5. Nullability hell

   var callback: Callback? = null
   override fun onAttach(context: Context?) { // 1
       this.callback = context as? Callback // 2
   }
   ...
   override fun onClick(v: View?) { // 3
       (v?.tag // 4
               as? SomeObj)?.let { obj -> //5
           callback?.objClicked(obj) // 6
       }
   }

   Strict way:

   lateinit var callback: Callback
   fun onAttach(context: Context) {
       this.callback = context as Callback
   }
   ...
   override fun onClick(v: View) {
       callback.objClicked(v.tag as SomeObj)
   }

   Explicit strict way:

   fun onAttach(context: Context) {
       this.callback = context as? Callback
               ?: throw ClassCastException("host context must implement my Callback")
   }
   ...
   override fun onClick(v: View) {
       callback.objClicked(
               v.tag as? SomeObj
                       ?: throw AssertionError("view $v expected to have a tag of type SomeObj, got ${v.tag}")
       )
   }

   Exception to this rule is where everything really can be nullable:

   private val PsiReference.outerMethodName: String? get() {
       val el = element
   
       // Java
       PsiTreeUtil.getParentOfType(el, PsiMethodCallExpression::class.java)?.methodExpression
           ?.takeIf { it.qualifierExpression === this }
           ?.let { (it.reference?.resolve() as PsiMethod?)?.name }
           ?.let { return it }
   
       // Kotlin
       PsiTreeUtil.getParentOfType(el, KtDotQualifiedExpression::class.java)
           ?.takeIf { it.receiverExpression.references.any { it.element == el } }
           ?.let { (it.selectorExpression as? KtCallExpression)?.calleeExpression?.references }
           ?.forEach { (it.resolve() as? PsiMethod)?.name?.let { return it } }
   
       return null
   }

6. Type checking

   if (iterable instanceof Collection) { ...

   Even Java stdlib:

   public static <T> List<T> unmodifiableList(List<? extends T> list) {
       return (list instanceof RandomAccess ?
               new UnmodifiableRandomAccessList<>(list) :
               new UnmodifiableList<>(list));
   }

   Perfectly valid code and bad code which breaks it. Oops! Even equals() is broken thanks to typecasting. equals(Object) is wrong; interface Comparable<T> { int compareTo(T) } is great and right.

   Kotlin version with 'safe cast':

   val Iterable<*>.size: Int
       get() = (this as? Collection<*>)?.size ?: iterator().size
   
   val Iterator<*>.size: Int
       get() {
           var count = 0
           while (hasNext()) {
               next(); count++
           }
           return count
       }
   

   but it's not safer!

7. data classes with no reason

   data class User(val name: String, val age: Int)

   The compiler automatically derives the following members from all properties declared in the primary constructor:

   • equals()/hashCode() pair;
   • toString() of the form "User(name=John, age=42)";
   • componentN() functions corresponding to the properties in their order of declaration;
   • copy() function (see below).

8. Destructuring of a non-tuple

   val (name, age) = user

9. Labeled return

   args.forEachIndexed { idx, arg ->
       visitParameter(
           params.getOrNull(idx)?.second ?: return@forEachIndexed,
           when (val it = arg.type) {
               PsiType.NULL -> null
               is PsiClassType -> it.resolve() ?: return@forEachIndexed
               is PsiPrimitiveType -> it.getBoxedType(arg)?.resolve() ?: return@forEachIndexed
               else -> return@forEachIndexed
           },
           arg.endOffset,
           collector
       )
   }

   Healthy person's return from anonymous:

   args.forEachIndexed(fun(idx: Int, arg: PsiExpression) {
       visitParameter(
           params.getOrNull(idx)?.second ?: return,
           when (val it = arg.type) {
               PsiType.NULL -> null
               is PsiClassType -> it.resolve() ?: return
               is PsiPrimitiveType -> it.getBoxedType(arg)?.resolve() ?: return
               else -> return
           },
           arg.endOffset,
           collector
       )
   }) 

10. open classes and functions

    Kotlin defaults are better! 'allopen' is a workaround.

    • default method emulation — OK

      /**
       * Simple adapter class for [TextWatcher].
       */
      open class SimpleTextWatcher : TextWatcher {
          /** No-op. */ override fun afterTextChanged(s: Editable) {}
          /** No-op. */ override fun beforeTextChanged(s: CharSequence, start: Int, count: Int, after: Int) {}
          /** No-op. */ override fun onTextChanged(s: CharSequence, start: Int, before: Int, count: Int) {}
      }

    • 'template method' pattern — OK

    • sharing logic within private scope, class count/size economy — OK

11. DTO/VO with default values, default constructor, and/or mutable fields

    class User(
        var name: String? = null,
        var age: Int = 0,
    )

12. Single operation on a Sequence

    array.map(Element::something)
    vs.
    array.asSequence().map(Element::something).toList()

13. Multiple operations on a collection

    array
        .filter { it.isCool }
        .map(Element::something)
        .sortedBy(Element::size)
    vs.
    array
        .asSequence()
        .filter(Element::isCool)
        .map(Element::something)
        .sortedBy(Element::size)
        .toList()

    Oh, we have map+filter in one run and in-place sorting!

    array
        .mapNotNull { if (it.isCool) it.something else null }
        .sortBy(Element::size)

14. interface Constants

    public interface Constants {
        int A = 1;
    }
    public class Whatever implements Constants {
        ...
    }

    const val A = 1;

    Nope! Constants are implementation details. They should be private or passed via constructor!

15. Enum as a stupid constant

    Bad:

    enum Role {
        User, Admin
    }
    // ...
    @DrawableRes int iconOf(Role role) {
        switch (role) {
            case User: return R.drawable.icon_role;
            case Admin: return R.drawable.icon_admin;
            default: throw new AssertionError();
        }
    }

    Better:

    enum Role {
        User(R.drawable.icon_role),
        Admin(R.drawable.icon_admin),
        ;
     
        @DrawableRes public final int icon;
    
        Role(@DrawableRes int icon) {
            this.icon = icon;
        }
    }

    OK for Kotlin, thanks to exhaustive when:

    enum class Role {
        User, Admin
    }
    // ...
    @DrawableRes fun iconOf(role: Role): Int = when (role) {
        Role.User -> R.drawable.icon_role
        Role.Admin -> R.drawable.icon_admin
    }

16. Reflect

    Given Java/Kotlin enum serialized as enum:

    enum class Role { USER, ADMIN }

    ...evolves to...

    sealed class Role {
        object User : Role()
        object Admin : Role()
    }

    um, where are SCREAMING_CASE names? will it break serialization? Healthy enum representation for serialization:

    mapOf(
        "USER" to Role.User,
        "ADMIN" to Role.Admin,
    )

    or

    listOf(Role.User, Role.Admin), Role::asString

    ---

    @JsonAdapter(DateAdapter.class)
    Date date1;
    
    @JsonAdapter(DateAdapter.class)
    Date date2;

    ...and after some time we have two date formats...

    class DateAdapter1 : TypeAdapter by DateAdapter(SimpleDateFormat("format #1"))
    class DateAdapter2 : TypeAdapter by DateAdapter(SimpleDateFormat("format #2"))

    Healthy code never mandate components to have no-arg constructor!

17. Implicit registration, e. g. Map<Class<T>, T>

    Java:

    • what about generic types?
    • runtime fail if required instance is not registered
    • nothing happens if useless instance is registered
    • different instances of the same type for different cases?

    Kotlin Map<KType, instance>:

    -what about generic types?
    +wildcards and projections?

    In the wild: Gson TypeAdapters, Jackson modules, DI containers, classpath scanning.

18. Annotation processing

    Code generation is same reflection, but at compile-time. This implies:

    − it does not work with separate compilation

    + it is more type-safe

    − larger bytecode

    + faster bytecode

19. Property delegation, an interesting but risky case of compile-time reflection

    class User(map: Map<String, Any>) {
        val name: String by map
        val age: String by map
    }

    Symbol name becomes a String! Refactoring breaks serialization ABI.

    Providing serialized name explicitly, just a fantasy:

    class User(map: Map<String, Any>) {
        val "name": String by map
        val "age": String by map
    }
    class User(map: Map<String, Any>) {
        val firstName: String by map as "name"
        val yearsAlive: String by map as "age"
    }

20. Object identity

    if (a == b) // Java

    if (a === b) // Kotlin

    Can be useful as an optimization, but could also indicate a design failure.

21. DSLs

    DSLs are useful for streaming (kotlinx.html) or wrapping legacy APIs (like Android Views: Anko, Splitties, etc). But if you need to create an object graph, just do it: cann constructors and pass objects there, without any wrappers, like Flutter UI framework does.

22. Mappers

    Smth smth = new Smth();
    smth.setA(someDto.getA());
    smth.setB(someDto.getX().getB());
    smth.setC(someDto.getX().getC());

    Smth(
        a = someDto.a,
        b = someDto.x.b,
        c = someDto.x.c
    )

    Solution: fix deserializer, SQL query, or whatever.

23. Convenience methods and overloads

    Imagine a TextView which has setText(CharSequence) method. That's okay.

    Now we add setText(@StringRes int resId) version which just does setText(res.getText(resId)). Why this is bad?

    • SRP is broken. Now you're not only a TextView, you're also an, um, ResourcesAccessor.
    • We're just on our way to add more convenience methods. What about setText(@StringRes int resId, Object... formatArgs) and setQuantity(@PluralsRes int id, int quantity, Object... formatArgs)?
    • You use a TextView somewhere, maybe in a dialog builder. Should you support all these overloads in your interface, too? Or maybe find which one is primary and which are convenience?

    Another bloated examples from Android SDK:

    • View.setBackground: setBackgroundColor, setBackgroundResource
    • ImageView.setImageDrawable: setImageResource, setImageURI, setImageIcon, setImageBitmap

    So, where are setBackgroundBitmap or setImageColor?

    Kotlin: it's okay to add convenience extension functions:

    • they don't bloat the class
    • more overloads can be added without editing class sources
    • in IDE, extensions can be easily distinguished from member functions by their appearance