Search code examples
javaooploose-couplingcohesion

Improving Cohesion and Coupling of Classes


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.


Solution

  • 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