A little help deciding where the business logic goes?

[Jon Swanson]

I'm trying to refactor some code that works, but where I think I can learn a lot by setting things up better. The part I am looking at is a panel that has several tabs on it. Exactly, what goes on each tab is related, but not the same in every tab. The basic idea, is that I build the tabs and assign each one an actionListener. When the user changes something on the tab, that tabs actionListener is called, which updates an Observable. There are a couple of other panels that use the information, that are registered as observers and they do their stuff when the Observable tells them to update.

Since my tabs are similar, I am building them like this-

   MyTabs tabOne = new MyTabs().labels(tabOneLabels()).fields(tabOneFields()).build();
   MyTabs tabTwo = new MyTabs().labels(tabTwoLabels()).fields(tabTwoFields()).build();

   private String[] tabOneLabels()
   {
      String[] labels = new String[NROWS];
      labels[0]  = null;
      labels[1]  = "Number";
      labels[2]  = "Choice";

      return labels;
   }

   private Container[] tabOneFields()
   {
      Container[] fields = new Container[NROWS];
      fields[0] = null;
      fields[1] = new JTextField(6);
      fields[1].addActionListener(new tabOneListener());
      fields[2] = new JComboBox(TAB_ONE_OPTIONS)
      fields[2].addActionListener(new tabOneListener());

      return fields;
   }

Now, the old code has an actionListener something like this-

   class tabOneListener implements ActionListener
   {
      @Override
      public void tabOneListener(ActionEvent e)
      {
         double number;
         String  choice;

         try
         {
            number = Double.parseDouble(tabOneNumber.getText());
         }
         catch (Exception en)
         {
//
// Do not display if we don't have a valid number
             observable.setTabOne( null );
         }
         try
         {
            choice = (String) tabOneCombo.getSelectedItem();
         }
         catch (Exception ec)
         {
//
// Do not display if we don't have a valid choice
             observable.setTabOne( null );
         }
         if (choice.equals("None")
         {
//
// take the default action
            observable.setTabOne( number );
         }
         else
         {
            observable.setTabOne( number, choice );
         }
      }
   }

Obviously the real code has a lot more logic in it.

The variables tabOneNumber and tabOneChoice currently are private variables of the class. I could continue that method of doing things by setting them in the tabOneFields method.

But since I am changing things, I would like to make as many improvements as I can. What I am not sure about is-

1. Should the actionListener be doing this much? Or should it just loop over all the fields and send a block of text to the observer and let it figure things out. My concern there is that the more I go from explicit variables like tabOneNumber to Container[1] the harder I think the code will be to read, especially if this is passed between classes, i.e., the observer gets String[] tabOneData and the methods make reference to tabOneData[0] or tabOneData[1], whereas now I might have a method updateTabOneNumber().

2. Am I making enough changes? The old code just called explicit makeTabOnePanel(), makeTabTwoPanel() methods and since I am having to both change the number of rows in the tabs and add a new tab, I wanted to be more flexible in this round. The observer keeps a data model that is used to compute the output. The methods the GUI calls tells it how to update the data model. It doesn't seem right (to me anyway) to have a class that has a method like createGUIFields() that I would pass to the observer which would use other methods to extract data. I would like to keep it clear what is used and when. Just not sure how.

Suggestions on how to think about this are most welcome.

[Kemal Sokolovic]

I'm trying to refactor some code that works, but where I think I can learn a lot by setting things up better.

Are you sure this code can be compiled? I have never heard of tabOneListener(ActionEvent e) method of ActionListener interface.

[Jon Swanson]

What I provided was only intended to give the idea of what I was doing. The code I am trying to rework is about 5000 lines. Since the problem isn't fixing a bug, I was trying to keep the question short and focus more on getting help with design philosophy for this type of problem rather than getting too focused on a big piece of code.

[dennis deems]

Ideally, the code that performs business logic is completely independent from GUI code. The convention is to call the layer with the business code (logic and state) the "Model", and the layer with the GUI code the "View". Code that communicates between the Model and View is called the "Controller" or "Presenter". If you ask ten different developers what the difference is between a controller and a presenter you'll get ten different answers, and it doesn't really matter that much. What's important is that this is the piece of code that carries information back and forth between the other layers.

The controller can be as slight or as substantial as you wish. Some developers make the controller little more than a messenger. The model is where the action is when you have this kind of controller. The controller tells the model that the view changed, then the model performs business logic and updates its state. Then the controller tells the view that the model has changed. The trick with this style of controller is to not get into an endless cycle. Also, if you have long-running operations like service calls from the model, this style makes it harder to do things like show progress to the user.

Another approach is to make the model a passive receptor of data, and to put all the heavy lifting in the controller. You still want the state and logic to live in the model layer, but it's the controller keeping track of everything. With this style of controller, you don't have to worry about update events triggering listeners unexpectedly, because the controller oversees all the updates itself.

Personally I favor the second style, but the important thing is consistency and the integrity of the model and view. Either way, I think it's better for the listener to hand off its information ASAP. I write my listeners very compactly. They call methods on the controller which set things in motion, and then they're done. But some devs prefer their listeners to accomplish more stuff. You can do that, but keep in mind that the more responsibility you give any single listener, the less opportunity you have to direct and guide things from the controller.

A key idea to take from this is that your business code doesn't need to know anything about tabs or containers on the GUI. It only needs to know about data. And the GUI doesn't need to know the significance of the data. It only needs to know what data to collect and how.

Hope this helps. Take a shot at implementing this kind of 3-tiered structure and let us know how it goes.

[Jon Swanson]

Thanks Dennis,

One thing is tripping me up is that I have a data model that is independent of the GUI. I do not have a perfect mapping between items in the data model and the GUI. This is because the user wants some of the GUI items to be used to convey several pieces of information and I prefer the model to cleanly delineate the information it is holding.

So I may type a value in a field that triggers an update to the model, but need to access three fields to produce the value that is recorded in the model. I don't want to tie the model to a specific way of entering the data. At the moment, the actionListener does the preprocessing and then updates the model.

The GUI has a tabbed panel that provides three ways to enter the data. I thought I was being clever, I made a builder that takes arrays (Components[]) for example, that allows me to populate each tab with different widgets (text field, combobox, etc) but all in the same consistent overall format.

Then I got to the actionListeners that originally (like the tabs) were hard-coded for each input method. From what you suggested, I think the actionListeners should preprocess the data to a common format supported by the data model.

What I haven't quite figured out is:

1. If I have a tabOneComponents = makeTabOneComponents(); how to then write the tabOneActionListener so that it knows what Component[1] really means in a way that if Component[1] gets a different meaning (or even different widget type) I don't have to run all over the code changing things.

2. The best way to map the calls to the methods of the observable to the preprocessing in the actionListener. Write now the actionListener needs to know the data types used in the model so it can send the right datatype, which might be OK. I toyed with just sending text, but then the observable would presumably do the preprocessing, which would require it know about how the GUI was laid out, which seems like a bad choice.

So maybe I shouldn't have said business logic, but there is still some logic that needs to interface between my nice clean data model and the unique ways that the user might want to express themselves. I want to make it as easy as possible to modify the fields and associated logic next time the user changes their mind about the best way to enter things, but right now its split into a builder piece for the actual GUI and the preprocessing in the listeners.

Is it clearer now where I am having trouble in the design?

[dennis deems]

I think coding listeners according to input method is better than having a general listener that then must sort out what kind of event it has received and what it must do with it. When I build a GUI I create as many listeners as I have input fields on the form. Each listener is an implementation designed to handle events from a specific widget, and knows what kind of data it's going to handle. I think this is cleaner and more manageable. Yes, this does mean that if the widget is changed then the listener must change. I would rather have a listener coded to a specific type that I might have to throw away sometime in the future, than an all-purpose listener that has to perform a bunch of checks just to find out what we're dealing with.

If the data needed by the model isn't the same type given from the GUI component, and you can't easily convert from one type to the other in just a line or two, make a method in the controller that converts between the types. I think your Double parsing is fine.

Here's some code that illustrates my typical approach. Extrapolate this for however many controls there are on the form. Note that from the controller and model perspective, it doesn't matter which tab is active. The model will get updated just the same from whatever event is generated.

public class Model
{
    // a very simple class with a getter and setter
	Double number;

	public Double getNumber()
	{
		return number;
	}

	public void setNumber( Double number )
	{
		this.number = number;
	}
}

public class View
{
	private JTextField numberField;

    // instantiate and layout components

    // note that I protect access to the view components
    // anyone can listen to the number text field
    // but they have to go through the view to do it
	public void addNumberListener( ActionListener listener )
	{
		numberField.addActionListener( listener );
	}

	public String getNumberText()
	{
		return numberField.getText();
	}
}


public class Controller
{
	private final Model model;
	private final View view;

    // view and model passed into the constructor
	public Controller( Model model, View view )
	{
		this.model = model;
		this.view = view;

    // You could conceptualize the controller as a bunch of listeners handling events.
    // When the controller is instantiated, all the listeners are created and set on the view
		ActionListener numberListener = buildNumberListener();
		view.addNumberListener( numberListener );
	}

    // create the listener that will handle events on the number text field
	private ActionListener buildNumberListener()
	{
		ActionListener listener = new ActionListener()
		{
			@Override
			public void actionPerformed( ActionEvent e )
			{
                try
                {
    				Double number = Double.parseDouble( view.getNumberText() );
                // if I needed to do other things with the data before sending it to the model,
                // I could easily pass it to another method in the controller.
                // If I had merely instantiated the listener on the fly while I created my GUI component,
                // I wouldn't be able to do that nearly as easily.
    				model.setNumber( number );
                 }
                 catch
                 { // do the right thing here }
			}
		};
		return listener;
	}
}

I'm not sure what Observable in your example is, but I suspect calling that setTabOne method isn't necessary.

Hope this helps. It was easier to show in code than explain with verbiage.

[Jon Swanson]

Dennis,

Thanks for the example. FYI the program was implemented with an Observer/Observable pattern rather than a Model-View-Controller pattern. So there will be some differences. When the observable is updated (it holds the model) it sends a signal to its registered observers and then they update themselves. The observer is GUI independent, at least as I have implemented it. The observers handle all the output, that part is nice and clean. Right now my view handles the controller parts like convert a text field to a number (GUI dependent) and the observable handles the GUI-independent parts, like calculate a regression from the model data. So where you have model.xxx I have observable.xxx and I was setting all the tab's values in one call, since observable was firing an update at the end of the set method. I may rethink that as a result if this discussion.

If there are 6 tabs, any one can be used to set the model (which is supposed to reflect the active tab as I have it now). Which is another design decision- keep fields for all the tabs in the model or use the GUI as the 'memory' of the state of a given tab.

This discussion and examples have been very helpful getting my head straight on this. I've not been doing OO long enough to understand the longer term costs/benefits of design decisions I make. But I am trying not to end up with too many future headaches.

[dennis deems]

Observer/Observable is a small-scale pattern. I don't think it's meant to be applied at the application level; unless the application is really simple there are likely to be things happening that aren't comfortably accommodated. In the case where the user is filling out a form, there will be form events that require handling. So isn't the form an observable, and don't we need observers handling those events? Suppose the application is a shopping cart. The user selects an item and enters a quantity, then the model behind the shopping cart needs to be updated. And probably the model is holding a subtotal, sales tax, shipping, and grand total; those values will change, and then the form needs to be updated to reflect that. So really we have observers in both directions. If we're not careful, these observers can get in each others' way, and the MVC pattern is one way to stop that from happening.

I've done a variation on MVC where the model was updated all at once from the GUI after the user pressed a Save button; it sounds like what you're doing with your tab update might be analogous to that. But without a controller entity that can go back and forth between the GUI and the model, you might find it difficult to avoid giving someone information that isn't really any of their business.

If the data being entered on each tab really is different then yeah, it's going to be cleaner if they get their own models. But ultimately I think you'll want that to feed up to another data structure right?

[Jon Swanson]

Actually, using the observer pattern was a suggestion from someone else at Java Ranch and it has worked out fairly well.

This discussion has helped clear up a number of things. I am thinking along these lines-

1. There is a fundamental model that is independent of the way the GUI gets its information. For example, the model might need a weight. That might be entered explicitly or the user might use a pull-down of things whose weights are known. The model and the logic it implements should just care that it gets a weight.

2. The GUI allows choices for entering data, each one has its own logic for turning that into a weight. I want that separate from the model, and I could use a controller to provide the specific translation between what the user provides and the 'fundamental' values stored in the model and used by methods in the model.

3. I think I will still use an observable, since I have many observers and may add more, it saves me from knowing ahead of time what might be displaying results and I can implement each as a separate class. So my controller will update the observable, rather than the model directly.

One other question. In your simple example, there is one view and it just handles a single field. I've got a JTabbedPane with what could be a growing number of tabs. My thought would be to create tab views and controllers for each tab. In fact, I actually have multiple JTabbedPanes feeding into the observable, which handles letting the observers know if it is 'complete.' I'm thinking if I have a tabOneView and tabOneController, I could just delete tabOne if they decided they didn't want it or completely redo it with no repercussions to the other tabs. I've got one other slight problem, that sometimes, information from a second JTabbedPane must be used when a value in the first JTabbedPane is changed. I'm trying to avoid keeping track of n x m combinations and would prefer to have m + n independent tab classes and my data model. That seems to argue for something between the tab logic and the model logic. It's in the observer right now. I'm not in control of the GUI layout and have been going nuts trying to organize the components into independent subsets without much luck.

I'd appreciate suggestions about the last set of problems. If I'm unclear, let me know.