DEV Community

Discussion on: Refactoring: My 6 favorite patterns

Collapse
 
marianban profile image
Marian Ban • Edited

Hi thanks for the article.
Regarding 1. Replace Conditional with Polymorphism I would rather think carefully before introducing PizzaUser and IceCreamUser for this concrete example. I know that this is probably not a real world example but I would rather point out few things before people start to go wild with inheritance 😃

So lets consider a new requirement: As an user I would like to receive emails for my favorite music and sms notifications for my favorite tv shows. So we have user.favorites.music, user.favorites.food, user.favorites.tvshow.

With our current implementation we are running into an obvious issue because we need something what is capable of sending pizza emails, favorite music emails and sms notifications. If we stick to our inheritance model then we will end up with many different classes like: (PizzaNirvanaGOTUser, IceCreamDoorsNarcosUser...). But we can do better with the command pattern 🚀. Example:

class User {
  constructor(user) {
    this._user = user;
  }
  notify(notificationSender) {
    notificationSender.execute(this);
  }
}

class CompositeNotificationSender {
  execute(user) {
    this._notificationSenders.forEach(x => x.execute(user));
  }
}

// concrete notification senders
class FavoriteFoodMailSender { ... }
class FavoriteMusicMailSender { ... }
class FavoriteTvShowSmsSender { 
  execute(user) {
    sendTvShowSms(user);
  }
}

// actual usage
const user = getUser();
const notificationSender = new CompositeNotificationSender(
  new FavoriteFoodMailSender(),
  new FavoriteMusicMailSender(),
  new FavoriteTvShowSmsSender()
); 
user.notify(notificationSender);

Happy coding!

Collapse
 
richardcochrane profile image
Richard Cochrane

That's a good point Marian and the crux behind the composition vs inheritance thinking but I think there's a place for both. My main decider (and it's beside the point to the one the author is making) is asking if that method really belongs on the class. So classes inheriting from Animal (Dolphin, Cat, Elephant) would all "make sound" (their own sound) so that would be a great case for inheritance but if the functionality is not related, eg. "send mail" (sorry, it's contrived but let's say in a talking animals universe, like Zootropolis, where animals can send letters either by running, flying or the postal service), the sending of emails is not intrinsic to being an animal and the same functionality could be rightfully applied to companies, people and other types of class that don't inherit from Animal, then composition is a better way to go. In my opinion, of course :-)

Collapse
 
gkatsanos profile image
George Katsanos • Edited

reading both the original recommendation as well as the alternative, I really have a strong feeling to stick with the conditions. Also, why not move simply move/abstract the condition inside the sendmail function? The information we need is already inside the user object. I find creating the extra classes for each type of favorite food overengineered TBH. And adding one new favorite food type, means we need a whole new class for it?

sendEmail(user) { return `<div>Hey ${user.firstName}, we saw you like ${user.favorites.food}</div>` };