Search code examples
kotlinmodelarchitectureclean-architecturedata-class

Seeking Advice on Designing a Clean Data Class (Model) in Kotlin


I'm currently working on designing a neat data class (model) for an AlarmTime instance, which will play the role of a variable in the Alarm class. I've come up with two solutions, and I'd love some advice on figuring out which one vibes better with Kotlin conventions and is just generally a good fit for this situation. If these two ideas are way off base, I'd be super grateful for any examples or guidance you could share.

Thanks a bunch!

Solution1

data class AlarmTime private constructor(
    val hour: Int,
    val minute: Int,
) {
    fun toCombinedMinutes(): Int = hour * 60 + minute

    companion object {
        private const val MAX_COMBINED_MINUTES = 1439 // 23 * 60 + 59

        operator fun invoke(hour: Int, minute: Int) =
            when {
                hour >= 24 || hour < 0 -> throw IllegalArgumentException(
                    "Invalid hour value. Hour must be within the range of 0 to 23.")
                minute >= 60 || minute < 0 -> throw IllegalArgumentException(
                    "Invalid minute value. Minute must be within the range of 0 to 59.")
                else -> AlarmTime(hour, minute)
            }

        operator fun invoke(combinedMinutes: Int) =
            when {
                combinedMinutes > MAX_COMBINED_MINUTES || combinedMinutes < 0 ->
                    throw IllegalArgumentException(
                        "Invalid combined minutes value. " +
                                "Combined minutes must be within the range of 0 to 1439"
                    )
                else -> AlarmTime(
                    hour = combinedMinutes / 60,
                    minute =  combinedMinutes % 60
                )
            }
    }
}

Solution2

data class Hour private constructor (val value: Int) {

    companion object {
       operator fun invoke(hour: Int) =
           when {
               hour >= 24 || hour < 0 -> throw IllegalArgumentException(
                   "Invalid hour value. Hour must be within the range of 0 to 23."
               )
               else -> Hour(hour)
           }
    }
}

data class Minute private constructor(val value: Int) {

    companion object {
        operator fun invoke(minute: Int) =
            when {
                minute >= 60 || minute < 0 -> throw IllegalArgumentException(
                    "Invalid minute value. Minute must be within the range of 0 to 59."
                )
                else -> Minute(minute)
            }
    }
}

data class CombinedMinutes(val value: Int) {

    fun getHour(): Hour = Hour(this.value / 60)
    fun getMinute(): Minute = Minute(this.value % 60)

    companion object {
        private const val MAX_COMBINED_MINUTES = 1439 // 23 * 60 + 59

        operator fun invoke(combinedMinutes: Int) =
            when {
                combinedMinutes > MAX_COMBINED_MINUTES || combinedMinutes < 0 ->
                    throw IllegalArgumentException(
                        "Invalid combined minutes value. " +
                                "Combined minutes must be within the range of 0 to 1439"
                    )
                else -> CombinedMinutes(combinedMinutes)
            }

        operator fun invoke(hour: Hour, minute: Minute) =
            CombinedMinutes(hour.value * 60 + minute.value)
    }
}

data class AlamTime2 private constructor(val hour: Hour, val minute: Minute) {
    companion object {
        operator fun invoke(hour: Hour, minute: Minute) =
            AlamTime2(hour, minute)

        operator fun invoke(combinedMinutes: CombinedMinutes) =
            AlamTime2(combinedMinutes.getHour(), combinedMinutes.getMinute())
    }
}

Solution

  • I'd like to challenge your premise that a data class is a good fit for what you intend to do.

    While not stated explicitly, from your code it looks like that you want to make sure that invalid AlarmTime can't be created. This will not work with data classes because one of the things you get for free is the copy function which enforces the types of values but not the validation. In particular the following compiles and runs:

    AlarmTime(12,30).copy(minute = -1)
      .also { println(it) } // AlarmTime(hour=12, minute=-1)
    

    This is an issue with Solution 1 and even more so with Solution 2 - consider this example that sets minutes to -1:

    AlamTime2(Hour(12), Minute(30))
      .copy(minute = Minute(1).copy(value = -1) )
    

    Without specific additional requirements (e.g equals/hashcode), I'd go for the simplified variant of Solution 1

    class AlarmTime(val hour: Int, val minute: Int) {
    
        constructor(combinedMinutes: Int) : this(
            hour = combinedMinutes / 60,
            minute = combinedMinutes % 60
        )
        
        init {
            require(hour in 0..23) {
                "Invalid hour value: $hour. Hour must be within the range [0 .. 23]"
            }
            require(minute in 0..59) {
                "Invalid minute value: $minute. Minute must be within the range [0 .. 59]."
            }
        }
    
        override fun toString(): String = "AlarmTime(hour=$hour, minute=$minute)"
    }