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

rook

02/15/2019, 3:04 PM
Just looking for feedback on this Presenter abstract that I’ve recently converted to Kotlin and tweaked. https://gist.github.com/nukeforum/8bc40d330adb9558d2060f08ac87b0e6
s

snowe

02/15/2019, 4:13 PM
mostly looks fine. I’d change https://gist.github.com/nukeforum/8bc40d330adb9558d2060f08ac87b0e6#file-presenter-kt-L38-L45 to something like
Copy code
protected fun view(): V? {
    return view?.get() ?: { 
        Log.w("Presenter", "View is null for $TAG")
        null
    }
}
that’s not tested though.
r

rook

02/15/2019, 4:24 PM
Yeah, I thought about doing something like that, I just wasn’t sure about the readability of it
s

snowe

02/15/2019, 4:25 PM
it’s up to you. I’d at least pull the return out of the if. that won’t affect readability much at all, in my opinion it would improve it.
the elvis operator could cause readability to suffer so that just depends on your views
you could also pull the logging statement completely out of the function and use it elsewhere.
or you could use aop on that method to log. clean it up in several ways
r

rook

02/15/2019, 4:47 PM
I put the log in there intentionally to help us catch moments when we’re attempting to use the presenter during incorrect times.
But you’re right about pulling the return out of the if
s

snowe

02/15/2019, 4:49 PM
yeah since you need the log maybe do something with AOP. That will generalize to the rest of your framework as well.
r

rook

02/15/2019, 4:56 PM
sorry, maybe I’m just dense, but I don’t know what you’re referring to when you say “AOP”
s

snowe

02/15/2019, 4:58 PM
ah, sorry. AOP stands for aspect oriented programming. You can use it for things like automatic logging of certain function calls, injection or modification of requests, etc. here is a short article describing what I’m talking about. https://www.yegor256.com/2014/06/01/aop-aspectj-java-method-logging.html
👍 1
r

rook

02/15/2019, 5:01 PM
Thanks, I’ll check it out
d

dewildte

02/15/2019, 6:07 PM
I disagree, The Elvis operator is perfectly readable for Kotlin programmers. IMO idiomatic implementations are only unreadable to the inexperienced. A perfect opportunity to gain that experience.
If your team complains even after learning about the feature then maybe you can change it. Until then it's perfectly acceptable to use a tool for what it was designed to do.
r

rook

02/15/2019, 6:13 PM
Either way, the provided example doesn’t actually work since the lambda following the elvis operator is coerced to a
Nothing?
return
s

snowe

02/15/2019, 7:23 PM
@dewildte when I said it might not be readable, it wasn’t due to the elvis, but the multiline block coming after the elvis.
@rook you can
return null
instead, but like I said, I didn’t know if it would actually work, just was spitballing.
4 Views