Message class, with constructors for data from user and from database

Question

I'm fairly new to Kotlin, and am looking to slim down a data class.

It has two constructors, one for creation based on user input, and one for creation based on database data.

open class Message : Unique {

    val timestamp: Long
    val contents: String

    constructor(contents: String) : super() {
        validateMessageContents(contents)
        this.contents = contents
        this.timestamp = System.currentTimeMillis()
    }

    constructor(id: String, contents: String, timestamp: Long) : super(id) {
        validateMessageContents(contents)
        this.contents = contents
        this.timestamp = timestamp
    }

    companion object {
        private const val MAX_MESSAGE_LENGTH = 1024

        private fun validateMessageContents(contents: String) {
            if (contents.isEmpty()) {
                throw IllegalArgumentException("Contents cannot be blank/empty")
            }
            if (contents.length > MAX_MESSAGE_LENGTH) {
                throw IllegalArgumentException("Message too long - please limit to $MAX_MESSAGE_LENGTH characters.")
            }
        }
    }
}

The Unique superclass has a single id: String property, but will generate a random UUID if the default constructor is called.

I'm wondering - how can I slim down this class? I've fiddled with trying to add a primary constructor, turn it into a data class, but each route I've hit a hurdle.

The things that stick out:

1. I'm calling validateMessageContents() in both constructors, but can't move it to init because the contents isn't yet available.
2. The second constructor, aside from validating the contents, is just two ugly assignment statements.
3. The two codepaths to either super() or super(id) make it difficult for one of these constructors to be made primary.

Answer 1

First, let's address your primary concern: the two constructors.

If the code to generate the UUID is not overly complicated, we can extract it and use it in our constructor as a default value (I don't like this because it adds code redundancy but I guess this is the only way to consolidate both constructors).
The same applies to the timestamp.
Thus we can use a primary constructor with the more specific super-constructor.

open class Message(
    id: String = UUID.randomUUID().toString(),
    val contents: String,
    val timestamp: Long = System.currentTimeMillis()
) : Unique(id) {

Here we combined both constructors and kept the order of the parameters.
Now we can use the init-block as well to validate the contents of the message:

init {
    validateMessageContents(contents)
}

That's the majority of things one can do to slim the class down.

Keep in mind that you need to specify a named parameter contents now because the first positional parameter id has a default value.

Message(contents = "test")

Another thing to do is to annotate your functions according to their behavior, so annotate validateMessageContents with @Throws.
I've also taken this opportunity to convert the two if-statements to a when-block. It's functionally the same (a string can't be empty and 1024 characters long at the same time).

@Throws(IllegalArgumentException::class)
    private fun validateMessageContents(contents: String) {
        when {
            contents.isEmpty() -> {
                throw IllegalArgumentException("Contents cannot be blank/empty")
            }
            contents.length > MAX_MESSAGE_LENGTH -> {
                throw IllegalArgumentException("Message too long - please limit to $MAX_MESSAGE_LENGTH characters.")
            }
        }
    }

Answer 2

1. You can use require to reduce your validations:

   require(contents.isNotEmpty()) { "Contents cannot be blank/empty" }
   require(contents.length > MAX_MESSAGE_LENGTH) {
       "Message too long - please limit to $MAX_MESSAGE_LENGTH characters."
   }
   

2. In my opinion, the main factor in preventing you from having a clean primary constructor is indeed the two paths to the super class (via super() or super(id)) and having a super class define the id property is in fact what also prevents you from making this a data class. I recommend making Unique an interface or abstract class with no properties in its constructor. e.g.:

   interface Unique {
       val id: String
   }
   
   fun randomId() = "${UUID.randomUUID()}"
   
   data class Message(
           override val id: String, 
           val contents: String, 
           val timestamp: Long
   ) : Unique {
       init {
           require(contents.isNotEmpty()) { "Contents cannot be blank/empty" }
           require(contents.length > MAX_MESSAGE_LENGTH) {
               "Message too long - please limit to $MAX_MESSAGE_LENGTH characters."
           }
       }
   
       constructor(contents: String) : this(
               id = randomId(),
               contents = contents,
               timestamp = System.currentTimeMillis()
       )
   
       companion object {
           const val MAX_MESSAGE_LENGTH = 1024
       }
   }
   

Answer 3

I only know Koltin in passing, but would it be an option to make the id parameter of the constructor nullable? That way you'd only have one code path, however you may would have to use null during construction.

Combined with GiantTree's suggestion, something like this:

open class Unique(optionalId: String? = null) {
    val id: String = if (optionalId == null) createId() else optionalId

    companion object {
        fun createId() = "Generated"
    }
}

open class Message(id: String? = null, contents: String, timestamp: Long = System.currentTimeMillis()) : Unique(id) {

    init {
        validateMessageContents(contents)
    }

    companion object {
        private const val MAX_MESSAGE_LENGTH = 1024

        private fun validateMessageContents(contents: String) {
           // ...
        }
    }
}


fun main(args: Array<String>) {
    val m1 = Unique()
    val m2 = Unique("id1")

    val m3 = Message(null, "abc", 1) // or
    val m4 = Message(contents = "abc", timestamp = 1)

    val m5 = Message("id2", "xyz", 2)

    println(m1.id)
    println(m2.id)
    println(m3.id)
    println(m4.id)
    println(m5.id)
}