6

When building an Observer pattern, you need to define your events that the are broadcast and observed. Is it better to define an abstract Event class with multiple subclasses for each event, or to define a single enum, where each event is listed in the enum?

option 1:

public abstract class Event { }
public UserInputEvent extends Event {}
public NetwortkEvent extends Event{}

option 2:

public enum Event {
 USER_INPUT_EVENT,
 NETWORK_EVENT
}

I've seen examples of both:

I'm leaning towards option 2. A lot of my decision is based on the fact that I'm working in java, so this might be different for different languages:

  1. option 1 will require lots of if X instanceOf Y checks which looks ugly.
  2. option 2 will allow clean switch statements
  3. option 2 will list all possible events nicely in one location. I know most IDEs can show all subclasses of a class but this is less obvious.
  4. option 1 doesn't restrict the addition of more subclasses. If you are writing a library, any user of the library can subclass Event and clutter the code with more events. option 2 fixes the number of events until someone actually updates the enum
  5. option 1 allows for sub-sub events, which may be good or bad. (e.g. public TouchUserInputEvent extends UserInputEvent {}
  6. option 1 allows you to attach additional meta data to the event. IMO this can lead to cluttered code if your goal is simplicity.

as an example:

public UserInputEvent extends Event {
    int xLocation;
    int yLocation;
}

UPDATE: I ended up going with option 1. I did have a need to include additional data in the event and using a class was the only way to go. Plush @whatsisname was right, having to re-fetch the updated event data would be callback hell.

8
  • 3
    Using your example: if you're using an enum for the event, how would you communicate xLocation and yLocation? Aug 8 '17 at 15:07
  • My thinking is that you wouldn't communicate any values, only that the values (stored somewhere else) had updated and that they needed to be (re)fetched from that other place.
    – tir38
    Aug 8 '17 at 16:00
  • 7
    You already made up your mind about the pro's and con's. The weight of each of these points depends on the application you are developing and on your programming style. We can not make that evaluation for you.
    – Philipp
    Aug 8 '17 at 16:11
  • 1
    Have you considered separating the user input observer and the network observer into separate interfaces? It seems unlikely you'll have many classes that act upon both events. As a guess, I think you are mixing concerns.
    – JimmyJames
    Aug 8 '17 at 21:02
  • Are you designing for a single monolithic local application or are you planning ahead to integrate this application events with other systems ?
    – Machado
    Aug 9 '17 at 12:43
8

My vote goes to option 1.

Because: there are no clean switch statements. When you have enums and start switching over them - you open the doors of a very dark place. First there is one switch, then there is another, and another, ...

And pretty soon, your code base has a lot of knowledge around the meaning of the different enum constants spread out in zillions of places.

There is only one good use for switches - which also "why option 1 is better". You see, in good OOP, you do not ask an object about its state to then make a decision on that. You rather invoke methods on that object and let it do what needs to be done.

In that sense: you spend your energy thinking up the common interface of your abstract class. And then you don't care about which "case" you have - because you always always always just invoke the same methods - but you do it on different objects.

And coming from there: sometimes you need to switch - because you have a factory that needs to find out what specific class needs to be instantiated and returned to the client code.

Long story short: the OOP answer to such kind of problems is polymorphism, and very often: a state machine. switch statements on the other hand have no(t much) place in OOP. The point is: using polymorphism, you can write code that is easier to maintain and enhance. Because such code nicely supports the open/closed principle. Scattered switch statements do not. To the contrary: they often force you to modify code when you try to enhance functionality.

[ for the record: I am not talking about fancy pattern matching as possible in newer languages - that would be a separate discussion ]

13
  • There definitely are uses for switch blocks, and if you are using one in a factory to decide a class you've already made other serious mistakes. I also find your 2nd to last statement about state machines and discounting switch statements to be rather unexpected. Aug 8 '17 at 20:31
  • @whatisname If you receive events from an enum based source (e.g. raw win32 messages) then you basically have to have a switch somewhere. It that example, it tends to be part of the platform library, but it is still there
    – Caleth
    Aug 9 '17 at 7:45
  • @whatsisname Based on my own experience: whenever you have multiple switches for the same thing to do different things - then you are looking at a state machine in disguise. Then you step back and implement that state machine and your switch statements vanish.
    – GhostCat
    Aug 9 '17 at 7:46
  • Switch blocks are sooooo 1970. Use a table-mapping strategy. Map each value of an enum to a method or class, and execute from there. Throw an exception if you don't have one mapping defined.
    – Machado
    Aug 9 '17 at 12:44
  • @Machado And I would say: why build this mapping manually? That is where polymorphism kicks in ...
    – GhostCat
    Aug 9 '17 at 12:45
3

Option 2 is a C style way of doing things. It's great, when you're programming in C.

However, you're using Java, which expects an object-oriented design. If you look at how other GUI systems, which have event handlers like you're imagining, more often than not they subclass various Event classes to cater to different types of events. Look at the EventArgs class and its children in C# for an example.

You concern 1 expecting many instanceof checks are not encountered with a sensible design. Instead, you use callback functions, (or delegates in C#, or similar things in other languages), where the callback knows what kind of data is coming in. An event listener expecting keyboard events has functions call that are provided exclusively keyboard events. No checking of instanceof or anything like that.

option 1 allows you to attach additional meta data to the event. IMO this can lead to cluttered code if your goal is simplicity.

If you think that grouping relevant bits of data into classes and passing those around leads to 'clutter', you're going to be in for some surprises in your future. Just getting notification of the event, then interrogating for updated values is neither cleaner nor less cluttered. If you do that, you not only have to maintain your event notification pipeline, but you now also may have to maintain additional data access methods. Additionally, you've now introduced race conditions that can cause some ridiculously obnoxious, hard to debug errors.

If you still don't believe my claims, write some C Win32 app where you handle the message pump, switching on your incoming message and checking and casting your lParam and wParam parameters (Option 2), then reimplement the app in C#, which wraps the message pump into providing an interface like Option 1, and see which ends up being cleaner and simpler.

1
  • "Just getting notification of the event, then interrogating for updated values is neither cleaner nor less cluttered. " good point. i thought about it more and the listen-for-event-then-ask-for-updated-value would get crazy really fast.
    – tir38
    Aug 11 '17 at 20:00
0

If you are programming in java i would use

public interface Event {...}

because in java both, an enum and a class can implement an interface.

Which one to use is an implementation detail that can be changed any time.

Java enums can have methods, too. Unfortunately Enums are not expandable so that an inherited enum cann not add additional const valuses.

So i assume you will end with @GhostCat excellent argumentation and use a class.

I use this interface aproach to handle image metadata. I have implemented EXIF meta data as enums (as exif can be represented as an ordinal) and Xmp meta data as class (its represented by non-ordinal strings) both implementing the same Interface

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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