18

In our Rails application, we are adding notifications. Some of these are blocking: They stop the progress of whatever resource they are added on, because some information on that resource is missing.

Other notifications are simple notifications, and only provide information.

Today I had a discussion with another programmer on our team. I have created the inheritance structure like this:

enter image description here

He however, would rather like me to just add blocking as a boolean-returning method on each Notification, and specify a list of subclasses that are blocking inside the Notification parent class.

The difference between these approaches is not very large; in my approach one does not have to specify this list, keeping the root class cleaner. On the other hand, the special logic that happens in Notification::Blocking right now is not very large either.

What kind of abstraction is more suited for this problem?

Qqwy
  • 4,907

5 Answers5

37

You want to avoid base classes knowing about derived classes. It introduces tight coupling and is a maintenance headache because you have to remember to add to the list every time you create a new derived class.

It will also prevent you from being able to put the Notification class into a reusable package/assembly if you wanted to use this class in multiple projects.

If you really want to use a single base class, another way to solve this is to add a virtual property or method IsBlocking on the base Notification class. Derived classes could then override that to return true or false. You would have a single class solution without the base class knowing about derived classes.

17 of 26
  • 4,850
13

and specify a list of subclasses that are blocking inside the Notification parent class.

That looks very peculiar and is a particular code smell.

I would provide subclasses if you have differences in behaviour between the classes, and you want to treat all of these notifications in the same fashion (i.e. using polymorphism).

Brian Agnew
  • 4,686
7

As a converse to the existing answers, I would suggest that the boolean property is the best option if the mode that is to be used is required to be changed dynamically (e.g. via a configuration file that gives a list of which types are to be blocking and which aren't).

That said, a better design even in this situation might be to use a Decorator object.

Jules
  • 17,880
  • 2
  • 38
  • 65
1

I would say it depends on how much else is special about a blocking notification, though my first thought is to go with "both":

class Notification
 virtual Boolean Blocking{get return false;}

class BlockingNotification inherits Notification
 virtual overrides Boolean Blocking{get return true;}

That way, you can use n.Blocking or n is BlockingNotification (all in pseudo-code), although, if you're going to allow a class to implement a context-sensitive Blocking value, seeing as you'd have to check that value every time, the BlockingNotification class becomes less useful.

In any case, I agree with the other answers that you don't want the base class implementation of Blocking to have to know about the derived classes.

Mark Hurd
  • 343
0

Instead of making two base classes and multiple instances of each, make one notification class with a bool to indicate if the notification is blocking and any other information needed to communicate the notification to the user.

This allows you to use one set of code for processing and presenting notifications and reduces the complexity of your code.

Trisped
  • 192