HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Java MVC model for large scale GUI using annotations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mvcjavalargeforusingannotationsscaleguimodel

Problem

I'm trying to make a good MVC model using annotations, which I will use it in a large scale GUI project. I want to respect the maximum of rules and guidelines, and be able to decorrelate every parts of the MVC. The code must be beautiful, testable and maintainable. I wrote a sample project to show you my model. What do you think about it?

The launcher:

package example;

import javax.swing.SwingUtilities;

/**
 * Test class.
 * 
 * @author AwaX
 * @date Feb 10, 2015
 */
public class Example_MVC {

    public static void main (String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run () {
                final Model model = new Model();
                final Controller controller = new Controller(model);
                controller.showGui();
            }
        });
    }
}


The data model:

package example;

import java.util.Observable;

/**
 * Data model.
 * 
 * @author AwaX
 * @date Feb 10, 2015
 */
public class Model extends Observable {

    private int counter;

    /**
     * Instanciate a default data model.
     */
    public Model () {
        super();
        this.counter = 0;
    }

    /**
     * Notify data model observers with new update.
     * 
     * @param arg
     *            Allow to specify a parameter for the observer.
     */
    public final void notifyChanged (Object... arg) {
        setChanged();
        if (arg.length == 0) {
            notifyObservers();
        } else if (arg.length == 1 && arg[0] != null) {
            notifyObservers(arg[0]);
        } else {
            throw new IllegalArgumentException("Only one argument allowed");
        }
        clearChanged();
    }

    public void inc () {
        this.counter++;
        notifyChanged(this.counter);
    }

    public int getCounter () {
        return counter;
    }

    public void setCounter (int c) {
        this.counter = c;
        notifyChanged(this.counter);
    }
}


The Controller:

`

Solution

Wow. The idea is nice, but I'll say it clearly: You failed at creating a beautiful, testable and maintainable architecture. Miserably :(

But let's start with first things first:

import javax.swing.SwingUtilities;


This seems to be the first error you've made: Chose your technologies.

Swing has officially reached end-of-life status:

6. Is JavaFX replacing Swing as the new client UI library for Java SE?


Yes. [...] While we recommend developers to leverage JavaFX APIs as much as
possible when building new applications, it is possible to extend a
Swing application with JavaFX, allowing for a smoother transition.

This seems to be your first and probably most grave mistake in this application. But let's keep going.

Your goals were testable & maintainable code. (the beautiful is a different story).

For that you need to rigorously separate classes from each other to allow proper unit-testing, which should be the first step to take.

Your classes on the other hand are tightly coupled. Changing the implementation of one of View, Model or Controller will almost automatically require changes in at least one of the others, which in turn will require more changes. Simple refactorings quickly become maintenance nightmares that way.

But this wouldn't be CodeReview if there was no solution for this. Java supplies a convenient way to allow you to specify behavior of a class without actually implementing a class: Interfaces

There's two simple benefits to interfaces:

  • You can change implementation details in classes without having to worry all that much about other files, as long as you keep the specified contract intact.



  • You can easily mock them for clean unit-tests



This provides a much better testability and maintainability.

Which brings me to the next point on my list. You're using standard Java's Observable. I have a personal dislike against Java's Observable, since it's use is extremely limited.

For starters anything abstract you'd want to make an Observable has to be an abstract class, since interfaces can't inherit from classes, this in turn limits you as Programmer when trying to properly decouple your code for testing and clean maintainable code.

Luckily it's not that hard to find a way around this: Roll your own observable implementation and if you have the spare slot, just extend a DefaultObservable of your own making.

Now let's see what that would do to your code:

public interface Model implements MyObservable {
     // methods shared across all models
}


public interface View implements MyObserver {
    void showGui();
    void addListener();
    // maybe removal of listeners, other things all your views do
}


public interface Controller {
    void showGui();
}


and now your classes are free to be properly named and unit-tested.

But since this is mostly a design review I still have some minor and major nitpicks about the actual code you've written:

-
Program against interfaces, the listeners in your gui shouldn't be forced into an ArrayList<>. What if you'd wanted to switch for a CopyOnWriteArrayList<>? Additionally a List seldom makes sense for listeners. There's no listener that would require to be notified twice about an event. Choose the right collection for the right job and use a Set instead. It intrinsically doesn't allow duplicate entries.

-
try to avoid pack(). It can be quite the performance eater and significantly slow down your GUI loading. Instead properly lay out your components using an inherently resizable-friendly LayoutManager (if you insist on using Swing), or switch to JavaFX altogether which is somewhat more forgiving on that matter

-
Try to avoid deeply nested loops wherever possible (and try to avoid needless Reflection usage). Deeply nested loops can be significantly simplified by extracting the relevant parts into separate methods, which gives greater reusability

-
Your createGui()'s Javadoc is a lie. You don't register listeners, you register ButtonActions. Generally your javadoc is rather meager (well it's an example implementation mostly)

-
ButtonAction as well as @ActionCallback should have their own classfiles. This allows greater reusability and easier taking over into an actual project.

-
You could have simplified ActionCallback usage by using the magic-attribute of interfaces (namely value()), which allows the following:

@ActionCallback("action1")
public void action1() {
    // code here
}


-
Your scheduler is never cleaned up properly, you let the JFrame's EXIT_ON_CLOSE call System.exit and are happy. You shouldn't be.

-
You should strongly consider going for Java 8 as technology. It removes lots of boilerplate, especially with the Runnables.

Code Snippets

import javax.swing.SwingUtilities;
public interface Model implements MyObservable {
     // methods shared across all models
}
public interface View implements MyObserver {
    void showGui();
    void addListener();
    // maybe removal of listeners, other things all your views do
}
public interface Controller {
    void showGui();
}
@ActionCallback("action1")
public void action1() {
    // code here
}

Context

StackExchange Code Review Q#80234, answer score: 6

Revisions (0)

No revisions yet.