CODEWITHSUNDEEP
kotlinimmutability
LivBanana
LivBanana

Reputation: 381

How to limit the risks of mutability in this example?

My code is something that is similar to:

fun verify(problem,answer){
    val errors: MutableList<String> = mutableListOf()
    if (!condiditon_1) {
        errors.add("Condition_1 is not verified")
    }
    if (!condition_2) {
        errors.add("Condition_2 is not verified")
    }
    return errors
}

Here errors is a mutable list. I am trying to rewrite the program limiting the mutability

The first way is:

errorsImmutable = errors.toList()

and return the immutable list errorsImmutable to prevent modifying it from the exterior.

Another solution is:

fun verify(problem,answer){
    val errors: List<String> = emptyList()
    if (!condiditon_1) {
        val mutableList =  errorLogs.toMutableList()
        mutableList.add("Condition_1 is not verified")
        errors = mutableList.toList()
    }
    if (!condition_2) {
        val mutableList =  errorLogs.toMutableList()
        mutableList.add("Condition_2 is not verified")
        errors = mutableList.toList()
    }
    return errors
}

Which one is better. More generally, is there an even better way to do it?

Upvotes: 0

Views: 75

Answers (2)

gidds
gidds

Reputation: 18597

In general, if you can create a collection already populated, that tends to be better than creating an empty one and then populating it — for a variety of reasons, including immutability as you say.  It's not always possible (or elegant), of course, but in this case here's one way:

fun verify() = listOfNotNull(
    if (condition1) "Condition1 is not verified" else null,
    if (condition2) "Condition2 is not verified" else null,
)

or the equivalent:

fun verify() = listOfNotNull(
    "Condition1 is not verified".takeIf{ condition1 },
    "Condition2 is not verified".takeIf{ condition2 },
)

That way you never see a mutable reference to the list (though one may be involved inside the listOfNotNull() implementation).  And because the list is now a single expression, you can use an expression-body function, and let it infer the type.  (Shorter code isn't always simpler or clearer or more maintainable, of course, but here I think it might be — depending on the surrounding code, of course.  Either way, it's often a useful learning exercise.)

And here's a more declarative approach, which defines the conditions separately (and factors out the common message format):

val conditions = listOf(
    Pair({ condition1 }, "Condition1"),
    Pair({ condition2 }, "Condition2"),
)

fun verify() = conditions.filter{ it.first() }
                         .map{ "${it.second} is not verified" }

PS. I missed another tweak: it might be slightly more readable if you use to instead of specifying Pair explicitly:

val conditions = listOf(
    { condition1 } to "Condition1",
    { condition2 } to "Condition2",
)

Upvotes: 0

Tenfour04
Tenfour04

Reputation: 93739

You neglected to show the return type of the function in the example. It is generally considered fine to simply return a read-only List and trust that consumers will not cast it to a MutableList to modify it.

fun verify(): List<String> {
    val errors: MutableList<String> = mutableListOf()
    if (!condiditon_1) {
        errors.add("Condition_1 is not verified")
    }
    if (!condition_2) {
        errors.add("Condition_2 is not verified")
    }
    return errors
}

There isn't a clean way to create a truly immutable List without writing your own List class that is immutable. Note that the standard library's listOf function returns read-only lists, not immutable lists, with the exception of the overload that has a single element argument.

Upvotes: 1

Related Questions