https://kotlinlang.org logo
#codereview
Title
# codereview
i

ivano

01/09/2020, 8:00 AM
Copy code
if (ocrItem.pattern is DateRegex) {
                        ocrItem.newValue = result.transformToDefaultDate()
                    } else {
                        ocrItem.newValue = result
                    }
a

adamd

01/09/2020, 9:13 AM
Copy code
ocrItem.newValue = result.let { if ocrItem.pattern is DateRegex it.transformToDefaultDate() else it }
I'm not sure if it is more readable after all but looks better to me
m

Matteo Mirk

01/09/2020, 9:47 AM
apart from readability, there’s a deeper design problem: you’re operating outside the object in an explicit way, against the “tell, don’t ask” principle (https://www.martinfowler.com/bliki/TellDontAsk.html): you’re asking the object for data and perform some logic outside instead of just telling it to do it for you. Following the principle your code should be something like:
Copy code
ocrItem.setResult(result)

// then, inside OcrItem class:
fun setResult(result: Result) {
    newValue = if (pattern is DateRegex) {             
        result.transformToDefaultDate()
    } else {
        result
    }       
}
(i made some things up as I don’t know the actual types you’re using)
💯 1
👍 2
Also, there’s nothing wrong or “javish” in using a good old if/else in Kotlin: most of the time makes the code way more readable than let/apply/also/?:
👍 1
i

ivano

01/09/2020, 11:03 AM
thank you very much
👍 1
2 Views