Should you pass an instance of service to a model class?

Question

Say we have these view model classes. You can imagine there's an entry in the UI for each subtype of InfoItem, and each item contains some text and an icon.

abstract class InfoItem(text: String, icon: Image)

class Email(..): InfoItem(..) {

    private email: String
}


class Map(..): InfoItem(..) {

    private address: String
}

Now, we need to handle click events. When clicking on an item, we should navigate to the appropriate screen. You can imagine that there's a component on its own that provides these routing methods.

interface Router {

    fun openEmailScreen(email: String)

    fun openMaps(address: String)
}

I can think of two ways to go about this.

First, we could have a separate function/component, that would do the appropriate thing based on the InfoItem type. However, in order to do that, we would have to make otherwise private properties of InfoType, public. For example:

fun route(infoItem: InfoItem) {
    when(infoItem) {
      is Email -> router.openEmailScreen(infoItem.email) <-- email must be public
      is Address -> router.openMaps(infoItem.address) <-- address must be public
    }
}

Another approach would be to introduce an abstract function navigate(router: Router) to InfoItem and have each item implement it separately. This way, each item knows what it needs to do and can keep all the details to itself. However, we now have a direct dependency to Router, which feels weird (although I can't explain why).

..
class Email(..): InfoItem(..) {

    private email: String

    fun navigate(router: Router) { router.openEmail(email) }
}
..

This example might be very simplistic, but I believe this issue applies to broader design as well.

Answer 1

Give your InfoItem a generic onClicked event. At the place (maybe a factory) where the specific InfoItem objects are created (and hence their type is known), assign each of them an appropriate event handler (so your EmailInfoItem gets the function router.openEmailScreen as event handler, and your MapInfoItem the function router.openMaps.

Now, when an item is "clicked", simply call the generic onClicked event.

Answer 2

No, models are models. To me they should only represent some piece of information, that's all. They shouldn't be the ones in charge of where to navigate to.

Why should a model know there's something like a Router anyways?

Besides that seems like a big logic redundancy having each item go like:

– «ok if I'm clicked I need to call this specific method on the router they gave me»

and then the Router go like:

– «ok they called this method on me so I need to show this screen»

You're moving routing logic to the models so that Router is more like a Navigator to me.

What I'd do is something like this:

open class InfoItem(val text: String, val icon: Image) {
    open val value: String? = null
}

class Email(val email: String, text: String, icon: Image) : InfoItem(text, icon) {
    override val value: String
        get() {
            return email
        }
}

class Address(val address: String, text: String, icon: Image) : InfoItem(text, icon) {
    override val value: String
        get() {
            return address
        }
}

interface Router {
    fun route(item: InfoItem)
}

Then in the ViewModel where you have your nice router: Router instance you have:

fun onItemClick(item: InfoItem) {
   this.router.route(item)
}