Constructor Design

1

We have a small argument in here and I was thinking to get the community opinion as well. Some say that this design is a bad design in Kotlin. In the end of the day, we just want to filter some data for our object. Is this indeed a bad design in Kotlin and should be avoided?

class Address(_addressLine: String = “Not Valid”) {

val addressLine1: String

init {
    if (_addressLine.length < 15) {
        this.addressLine1 = _addressLine
    }
    else this.addressLine1 = "lenght too big"

}
2

it is a valid design, though IDE probably proposed to replace it by

    addressLine1 = if (_addressLine.length < 15) {
        _addressLine
    } else {
        "length too big"
    }

Only two small remarks:

  • No need to use _ in parameter name. Since it is not val, you can use the same name for constructor parameter an class property even if they have different meaning
  • In Kotlin if is an expression, so you can write it like this:
class Address(addressLine: String = “Not Valid”) {
  val addressLine =  if (addressLine.length < 15) {
        addressLine
    } else {
        "length too big"
    }
}

And the last remark. It is much easier to use nullables for non-valid parameters.

3

I don’t think this is an issue of bad Kotlin design, as a simple mixing of concerns.

Either you’re mixing your inbound validation (that the user should not enter such a long address), or your outbound messaging (simply printing this instead of the address). Regardless, there’s no reason to store that message next to your address domain data - that message is purely a part of the view layer.

1 Like
4

How I’d write it:

class Address(val addressLine1: String) {
    init { require(addressLine1.length < 15, "Address line 1 length too large") }
}

Of course, I wouldn’t bury my user validation in my models. Also, not sure why “Not Valid” is there, because it actually does appear valid. There are better ways to represent invalid but accepted data than a stringly typed literal.

5

I would validate the length in the Adress class, too. But I would not store the error message (length too big) in a field! I would use a precondition like @cretz suggested or use a validation framework like Bean Validation. The advantage of the letter is that you can validate more than one parameter at once and that you can validate with different sets of constraints in different contexts. “valid for the next workflow step” is a completely different thing than “valid for shipping” for example, but both can be performed on the same instance.

6

In terms of Kotlin, this is a little odd but certainly allowed (see @darksnake comment).
However, in terms of software, this is bad.
Using a magic string value to represent an error rather that throwing an exception or using a sealed class to represent an error case is a bad idea in any language.