views:

179

answers:

6

I am working with a log of events where there are about 60 different "types" of events. Each event shares about 10 properties, and then there are subcategories of events that share various extra properties.

How I work with these events does depend on their type or what categorical interfaces they implement.

But it seems to be leading to code bloat. I have a lot of redundancy in the subclass methods because they implement some of the same interfaces.

Is it more appropriate to use a single event class with a "type" property and write logic that checks type and maintain some organization of categories of types (e.g. a list of event types that are category a, a second list that are category b, etc)? Or is the subclass design more appropriate in this case?

First Approach:

public interface Category1 {}
public interface Category2 {}

public abstract class Event {
 private base properties...;
}

public class EventType1 extends Event implements Category1, Category2 {
 private extra properties ...;
}

public class EventType2 extends Event implements Category3, Category4 {
 private extra properties ...;
}

Second Approach:

public enum EventType {TYPE1, TYPE2, TYPE3, ...}
public class Event {
 private union of all possible properties;
 private EventType type;
}

My personal opinion is that it seems like a single event object is what is appropriate, because, if I am thinking about it correctly, there is no need for using inheritance to represent the model because it is really only the behavior and my conditions that alter based on the type.

I need to have code that does stuff like:

if(event instanceof Category1) {
  ...
}

This works well in the first approach in that instead of instanceof I can just call the method on the event and implement "the same code" in each of the appropriate subclasses.

But the second approach is so much more concise. Then I write stuff like:

if(CATEGORY1_TYPES.contains(event.getEventType()) {
 ...
}

And all my "processing logic" can be organized into a single class and none of it is redundantly spread out among the subclasses. So is this a case where although OO appears more appropriate, it would be better not too?

+1  A: 

I would go with the object per event type solution, but I would instead group commonly used combinations of interfaces under (probably abstract) classes providing their skeletal implementations. This greatly reduces the code bloat generated by having many interfaces, but, on the other hand, increases the number of classes. But, if used properly and reasonably, it leads to cleaner code.

petr k.
The events implement multiple interfaces and java doesn't support multiple inheritance so i cannot subclass events in this manner. At least, I don't think I can.
Josh
I see your problem more clearly now. Then, you can reuse the skeletal implementations via object containment and delegate interface calls to those contained objects. Then again, the gain really depends on how much actual code needs to go to the interface methods implementation.
petr k.
A: 

Merely having a large number of .java files isn't necessarily bad. If you can meaningfully extract a small number (2-4 or so) of Interfaces that represent the contracts of the classes, and then package all of the implementations up, the API you present can be very clean, even with 60 implementations.

I might also suggest using some delegate or abstract classes to pull in common functionality. The delegates and/or abstract helpers should all be package-private or class-private, not available outside the API you expose.

James A. Rosen
A: 

If there is considerable mixing and matching of behavior, I would consider using composition of other objects, then have either the constructor of the specific event type object create those objects, or use a builder to create the object.

perhaps something like this?

class EventType {

  protected EventPropertyHandler handler;

  public EventType(EventPropertyHandler h) {
     handler = h;
  }

  void handleEvent(map<String,String> properties) {
    handler.handle(properties);
  }
}

abstract class EventPropertyHandler {
   abstract void handle(map<String, String> properties);
}
class SomeHandler extends EventPropertyHandler {
   void handle(map<String, String> properties) {
      String value = properties.get("somekey");
      // do something with value..
   }
}

class EventBuilder {
   public static EventType buildSomeEventType() {
      // 
      EventType e = new EventType( new SomeHandler() );
   }
}

There are probably some improvements that could be made, but that might get you started.

+1  A: 

Inheritance can be limiting if you decide to extend an abstract base class of a particular Category interface, because you might need to implement another Category as well.

So, here is a suggested approach: Assuming you need the same implementation for a particular Category interface method (regardless of the Event), you could write an implementation class for each Category interface.

So you would have:

public class Category1Impl implements Category1 {
    ...
}

public class Category2Impl implements Category2 {
    ...
}

Then for each of your Event classes, just specify the Category interfaces it implements, and keep a private member instance of the Category implementation class (so you use composition, rather than inheritance). For each of the Category interface methods, simply forward the method call to the Category implementation class.

pythonquick
Yep, pretty much exactly what I suggested in my answer and comment below. This approach proves to be way more flexible than inheritance-based approach.
petr k.
A: 

It depends on if each type of event inherently has different behavior that the event itself can execute.

Do your Event objects need methods that behave differently per type? If so, use inheritance.

If not, use an enum to classify the event type.

Scott Stanchfield
There are behaviors common to all events, there are behaviors common to categories of the events (common interfaces), and there are behaviors specific to events, and there are behaviors specific to properties of some events (these same properties can appear in different types of events)
Josh
+1  A: 

Since I didn't really get the answers I was looking for I am providing my own best guess based on my less than desirable learning experience.

The events themselves actually don't have behaviors, it is the handlers of the events that have behaviors. The events just represent the data model.

I rewrote the code to just treat events as object arrays of properties so that I can use Java's new variable arguments and auto-boxing features.

With this change, I was able to delete around 100 gigantic classes of code and accomplish much of the same logic in about 10 lines of code in a single class.

Lesson(s) learned: It is not always wise to apply OO paradigms to the data model. Don't concentrate on providing a perfect data model via OO when working with a large, variable domain. OO design benefits the controller more than the model sometimes. Don't focus on optimization upfront as well, because usually a 10% performance loss is acceptable and can be regained via other means.

Basically, I was over-engineering the problem. It turns out this is a case where proper OO design is overkill and turns a one-night project into a 3 month project. Of course, I have to learn things the hard way!

Josh