Improving Cohesion and Coupling of Classes

Question

I am given this set of code and need to suggest ways to improve the code's cohesion and coupling of the classes. But I thought these classes are quite well de-coupled since it looks like they are making use of events. And in terms of cohesion, all the init() calls are placed together, everything seems quite alright to me.

public class A
{
    private C t;
    private B g;
    public static void main(String args[]) {
        // Creates t and g.
        t = new C();
        t.init();
        g = new B();
        g.init();
        g.start(this);
    }
    pubic void update (Event e)
    {
        // Performs updating of t based on event
    }
}

public class C
{
    public C() { ... }
    public void init() { ... }
}

public class B
{
    public B() { ... }

    public void init() { ... }

    public void start(A s) {
        e = getNextEvent();
        while (e. type != Quit)
            if (e.type == updateB)
                update(e) ;
            else
                s.update(e) ;
        e = getNextEvent();
    }

    public void update(Event e) { ... }
    }
}

Are there still ways to improve the classes cohesion and coupling? It looks ok to me but I think I am missing something out.

Thanks for any suggestions on this.

Answer 1

One suggestion would be to decouple event handling logic from the controller logic (class A).

So you would have 4 types of classes:

• the main class used for running the "server" (A)
• the thread listening for events (B)
• the model layer which will be updated (C)
• an event handler class which will support some operation on an event (D)

It could look something like this:

public class Main {
  private Model m;
  private EventListener listener;

  ... main() {
    m = new Model();
    listener = new EventListener();

    EventHandler eventHandler = new MyEventHandler();

    // set the event handler
    listener.setEventHandler(eventHandler);

    listener.start(m);
}

public class Model {
  // nothing to see here
}

public class EventListener() {

  private EventHandler handler = new DefaultEventHandler();

  public void start(final Model m) {
    // startup
    while (event.type != Event.Quit) {
      // get event
      handler.handleEvent(event, m);
    }
    // shutdown
  }

  public void setEventHandler(EventHandler eh) { this.handler = eh }
}

public class MyEventHandler implements EventHandler {

  public void handleEvent(Event e, Model m) {
    // update the model
  }

}

Note that in this new design the business logic of updating the model (C in your example) has moved to an external class as opposed to the "Runner" class. this is a bit cleaner as the Main class does not need to have knowledge of what events are and how to handle them.

Another advantage is that using this you can easily code complex event handling using chained event handlers or multiple serial event handlers. It is also quite simple to implement asynchronous event handling as B is only in charge of calling the handlers and doesn't need to understand event types. This is sometimes referred to as Publish/Subscribe and keeps the listener (B) and the handler (your update(e) method) loosely coupled

Answer 2

Start writing unit tests (better yet, do TDD). The coupling (and, to a lesser extent, the cohesion or lack thereof) will become immediately evident.

For instance, class B's start method has a parameter of type A. In your example you could just instantiate A, but what if A had other dependencies? Perhaps all start needs is an interface that A implements (with an update method).

And what does getNextEvent do? If it uses other dependencies, getting B in a test harness may prove difficult.

Testing can help validate your design.

Answer 3

Without seeing more of your code it is difficult to say how decoupled your code is. Events are one way to decouple code, but can also make code more difficult to understand and debug.

When designing classes, high-cohesion means many of the methods reuse each other, and low-coupling means you only need to expose a few public methods.

When designing packages, high-cohesion means many of the classes in a package depend on each other, and low-coupling means only a few are public scope, or message with other classes through interfaces.

The benefits of high-cohesion, low-coupling should be less pain, especially when it comes to responding to change. If it doesn't reduce pain, don't spend a lot of time optimizing it. I know it sounds like I'm spouting platitudes here, but you should keep your own metric in mind when measuring whether high-cohesion, low-coupling is 'good enough' rather than relying on the opinions of people who do not necessarily understand the problem you are trying to solve.

Answer 4

If the original design is to incorporate another functionality or classes in the future, using an event handler, as you said, it has been used, then just concentrate on the strategy pattern on the implementation and the optimization on classes or interfaces.