10

Consider two classes Dog and Cat both conforming to Animal protocol (in terms of Swift programming language. That'd be interface in Java/C#).

We have a screen displaying a mixed list of dogs and cats. There's Interactor class that handles logic behind scenes.

Now we want to present a confirmation alert to the user when he wants to delete a cat. However, dogs need to be deleted immediately without any alerts. The method with conditionals would look like this:

func tryToDeleteModel(model: Animal) {
    if let model = model as? Cat {
        tellSceneToShowConfirmationAlert()
    } else if let model = model as? Dog {
        deleteModel(model: model)
    }
}

How can this code be refactored? It obviously smells

4 Answers 4

9

You're letting the protocol type itself determine behavior. You want to treat all protocols the same throughout your program except in the implementing class itself. Doing it this way is respecting Liskov's Substitution Principle, which says you should be able to pass either Cat or Dog (or any other protocols you might eventually have under Animal potentially), and have it work indifferently.

So presumably you'd add an isCritical func to Animal to be implemented by both Dog and Cat. Anything implementing Dog would return false and anything implementing Cat would return true.

At that point, you'd only need to do (My apologies if syntax isn't correct. Not a user of Swift):

func tryToDeleteModel(model: Animal) {
    if model.isCritical() {
        tellSceneToShowConfirmationAlert()
    } else {
        deleteModel(model: model)
    }
}

There is only a small problem with that, and that is that Dog and Cat are protocols, meaning they in themselves don't determine what isCritical returns, leaving this to every implementing class to decide for themselves. If you have a lot of implementations, it would probably be worth your while to create an extendable class of Cat or Dog which already correctly implements isCritical and effectively clearing all implementating classes from the need to override isCritical.

If this doesn't answer your question, please write in the comments and I'll expand my answer accordingly!

5
  • It's a little unclear in the statement of the question, but Dog and Cat are described as classes, while Animal is a protocol that's implemented by each of those classes. So there's a bit of a mismatch between the question and your answer.
    – Caleb
    Oct 4, 2018 at 3:29
  • So you suggest to let model decide whether to present a confirmation popup or not? But what if there's a heavy logic involved, like show popup only if there are 10 cats displayed? The logic depends on Interactor state now Oct 4, 2018 at 6:03
  • Yeah, sorry about the unclear question, I've made few edits. Should be more clear now Oct 4, 2018 at 6:10
  • 1
    This kind of behaviour should not be linked to the model. It depends on the context and not the entity itself. I think Cat and Dog are more likely to be POJO. Behaviours should be handle at other places and be able to change according to the context. Delegating behaviours or methods which behaviours will rely on in Cat or Dog will lead to too much responsibilities in such classes. Oct 4, 2018 at 12:41
  • @GrégoryElhaimer Please note that it isn't determining behavior. It is merely stating whether or not it is a critical class. Behaviors throughout the program which need to know if it is a critical class can then evaluate and act accordingly. If this is indeed a property which differentiates how instances in both Cat and Dog are handled, it can and should be a common property in Animal. Doing anything else is asking for a maintenance headache later.
    – Neil
    Oct 5, 2018 at 10:22
4

Tell vs. Ask

The conditional approach you're showing we would call "ask".  This is where the consuming client asks "what kind are you?" and customizes their behavior and interaction with objects accordingly.

This contrasts with the alternative we call "tell".  Using tell, you push more of the work into the polymorphic implementations, so that the consuming client code is simpler, without conditionals, and common regardless of the possible implementations.

Since you want to use a confirming alert, you could make that an explicit capability of the interface.  So, you might have a boolean method that optionally checks with the user and returns the confirmation boolean.  In the classes that don't want to confirm, they simply override with return true;.  Other implementations might dynamically determine if they want to use confirmation.

The consuming client would always use the confirmation method regardless of the particular subclass it is working with, which makes the interaction tell instead of ask.

(Another approach would be to push the confirmation into the deletion, but that would surprise consuming clients who expect a deletion operation to succeed.)

2
  • So you suggest to let model decide whether to present a confirmation popup or not? But what if there's a heavy logic involved, like show popup only if there are 10 cats displayed? The logic depends on Interactor state now Oct 4, 2018 at 6:02
  • 2
    Ok, yes, that is a different question, requiring a different answer.
    – Erik Eidt
    Oct 4, 2018 at 18:43
2

Determining whether a confirmation is needed is the responsibility of the Cat class, so enable it to perform that action. I do not know Kotlin, so I'll express things in C#. Hopefully the ideas are then transferable to Kotlin too.

interface Animal
{
    bool IsOkToDelete();
}

class Cat : Animal
{
    private readonly Func<bool> _confirmation;

    public Cat (Func<bool> confirmation) => _confirmation = confirmation;

    public bool IsOkToDelete() => _confirmation();
}

class Dog : Animal
{
    public bool IsOkToDelete() => true;
}

Then, when creating a Cat instance, you supply it with TellSceneToShowConfirmationAlert, which will need to return true if OK to delete:

var model = new Cat(TellSceneToShowConfirmationAlert);

And then your function becomes:

void TryToDeleteModel(Animal model) 
{
    if (model.IsOKToDelete())
    {
        DeleteModel(model)
    }
}
5
  • 1
    Doesn't this move the delete logic into the model? Wouldn't it be much better to use another object to handle this? Possibly a data structure like a Dictionary<Cat> inside an ApplicationService; check to see if the Cat exists and if it does then to fire off the confirmation alert?
    – keelerjr12
    Oct 3, 2018 at 11:51
  • @keelerjr12, it moves the responsibility for determining if a confirmation is needed for the delete into the Cat class. I'd argue that that is where it belongs. It doesn't get to decide how that confirmation is achieved (that is injected) and it doesn't delete itself. So no, it doesn't move the delete logic into the model.
    – David Arno
    Oct 3, 2018 at 12:04
  • 2
    I feel like this approach would lead to tons and tons of UI-related code attached to the class itself. If the class is intended to be used across multiple UI layers, the problem grows. However if this is a ViewModel type class, rather than a business entity, then it seems appropriate.
    – GHP
    Oct 3, 2018 at 12:44
  • @Graham, yes that's definitely a risk with this approach: it relies on it being easy to inject TellSceneToShowConfirmationAlert into an instance of Cat. In situations where that's not an easy thing (such as in a multi-layered system where this functionality lies at a deep level), then this approach would not be a good one.
    – David Arno
    Oct 3, 2018 at 13:07
  • 1
    Exacty what I was getting at. A business entity vs a ViewModel class. In the business domain, a Cat shouldn’t know about UI related code. My family cat doesn’t alert anyone. Thanks!
    – keelerjr12
    Oct 3, 2018 at 13:11
1

I would advice to go for a Visitor pattern. I did a small implementation in Java. I'm not familiar with Swift, but you can adapt it easily.

The visitor

public interface AnimalVisitor<R>{
    R visitCat();
    R visitDog();
}

Your model

abstract class Animal { // can also be an interface like VisitableAnimal
    abstract <R> R accept(AnimalVisitor<R> visitor);
}

class Cat extends Animal {
    public <R> R accept(AnimalVisitor<R> visitor) {
         return visitor.visitCat();
     }
}

class Dog extends Animal {
    public <R> R accept(AnimalVisitor<R> visitor) {
         return visitor.visitDog();
     }
}

Calling the visitor

public void tryToDelete(Animal animal) {
    animal.accept( new AnimalVisitor<Void>() {
        public Void visitCat() {
            tellSceneToShowConfirmation();
            return null;
        }

        public Void visitDog() {
            deleteModel(animal);
            return null;
        }
    });
}

You can have as much implementations of AnimalVisitor as you want.

Example:

public void isColorValid(Color color) {
    animal.accept( new AnimalVisitor<Boolean>() {
        public Boolean visitCat() {
            return Color.BLUE.equals(color);
        }

        public Boolean visitDog() {
            return true;
        }
    });
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.