Just looking for feedback on this Presenter abstra...

# codereview

[rook]

Just looking for feedback on this Presenter abstract that I’ve recently converted to Kotlin and tweaked.
https://gist.github.com/nukeforum/8bc40d330adb9558d2060f08ac87b0e6

[snowe]

mostly looks fine. I’d change
https://gist.github.com/nukeforum/8bc40d330adb9558d2060f08ac87b0e6#file-presenter-kt-L38-L45

to something like

protected fun view(): V? {
    return view?.get() ?: { 
        Log.w("Presenter", "View is null for $TAG")
        null
    }
}

[snowe]

that’s not tested though.

[rook]

Yeah, I thought about doing something like that, I just wasn’t sure about the readability of it

[snowe]

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.

[snowe]

the elvis operator could cause readability to suffer so that just depends on your views

[snowe]

you could also pull the logging statement completely out of the function and use it elsewhere.

[snowe]

or you could use aop on that method to log. clean it up in several ways

[rook]

I put the log in there intentionally to help us catch moments when we’re attempting to use the presenter during incorrect times.

[rook]

But you’re right about pulling the return out of the if

[snowe]

yeah since you need the log maybe do something with AOP. That will generalize to the rest of your framework as well.

[rook]

sorry, maybe I’m just dense, but I don’t know what you’re referring to when you say “AOP”

[snowe]

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

[rook]

Thanks, I’ll check it out

[dewildte]

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.

[dewildte]

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.

[rook]

Either way, the provided example doesn’t actually work since the lambda following the elvis operator is coerced to a Nothing? return

[snowe]

@dewildte when I said it might not be readable, it wasn’t due to the elvis, but the multiline block coming after the elvis.

[snowe]

@rook you can return null instead, but like I said, I didn’t know if it would actually work, just was spitballing.