Software design - wizard

[lancout]

Hi,

I am working on a project which has a large number of wizard type
forms. A great deal fall into the functional pattern of

a) Display a list of (business) objects
b) User selects from list and presses Next
c) Display a form to edit selected (business object)
d) Click apply or finish etc.

So I wrote some code to do that, for reference it is shown below.

I have a class 'FormWizard' which is reponsible for the basic UI frame,
selecting from a list, next etc.

I have a model 'FormWizardModel' which contains all the actions and
data FormWizard needs to work.

I have one implementation - a UserWizard which populates the wizard
model with the actions, and most importantly a standalone jpanel to
edit users.

Sound good? Maybe, I had a troubleshoot bugs a number of times which
usually indicates a weak design. The main problem was passing in null
values to the FormWizard due to the timing of when the Next button
ActionListener was pressed.

I resolved that but I am not overly happy as you have this shared
FormWizardModel which is populated at alternating times by either the
UserWizard or the FormWizard and that seems dangerous as it is
difficult using just code inspection to determine the state of the
FormWizardModel at any point in time. The reason it is written that
way though is primarily to serve seperating the code that will change
with each Wizard implementation.

This *MUST* be a common design pattern - can anyone help me improve
mine.

Here are my main classes skipping the ordinary getters and setters:

public class FormWizardModel {

private String screen1Title;
private String screen2Title;
private ActionListener finishAction;
private ActionListener nextAction;
private ResultList resultList;
private JPanel screen2Panel;
private String finishButtonText = "Finish";
private Object userObject;
private Dimension screen1Size = UIProperties.DIALOG_SIZE_450_330;
private Dimension screen2Size = UIProperties.DIALOG_SIZE_450_330;
}


public class UserWizard {

private FormWizardModel m = new FormWizardModel();
private FormWizard wizard;

public UserWizard(JFrame frame) {
wizard = new FormWizard(frame, m);
}

public void start() throws Exception {
m.setNextAction(getNextAction());
m.setFinishButtonText("Close");
m.setResultList(getList());
//TODO get this title from reference
m.setScreen1Title("Select user");
m.setScreen2Size(UIProperties.WINDOW_SIZE_600_400);
wizard.start();
}

private ResultList getList() throws Exception {
UserList users = SecurityServices.getUsers();
ApplicationControlList acl1 =
SecurityServices.getAppControlsByUser(ApplicationGroup.USERSEARCHRESULTS);
users.setColumnOrder(acl1);
return users;
}

private ActionListener getNextAction() throws Exception {
return new ActionListener() {

public void actionPerformed(ActionEvent e) {
UserModel rm = (UserModel) m.getUserObject();
m.setScreen2Title(rm.getDisplayName());
UserFormController userFormController = new UserFormController();
UserFormUI ui = userFormController.start(rm);
m.setScreen2Panel(ui.getMainPanel());
}

};
}

}




public class UserWizard {

private FormWizardModel m = new FormWizardModel();
private FormWizard wizard;

public UserWizard(JFrame frame) {
wizard = new FormWizard(frame, m);
}

public void start() throws Exception {
m.setNextAction(getNextAction());
m.setFinishButtonText("Close");
m.setResultList(getList());
//TODO get this title from reference
m.setScreen1Title("Select user");
m.setScreen2Size(UIProperties.WINDOW_SIZE_600_400);
wizard.start();
}

private ResultList getList() throws Exception {
UserList users = SecurityServices.getUsers();
ApplicationControlList acl1 =
SecurityServices.getAppControlsByUser(ApplicationGroup.USERSEARCHRESULTS);
users.setColumnOrder(acl1);
return users;
}

private ActionListener getNextAction() throws Exception {
return new ActionListener() {

public void actionPerformed(ActionEvent e) {
UserModel rm = (UserModel) m.getUserObject();
m.setScreen2Title(rm.getDisplayName());
UserFormController userFormController = new
UserFormController();
UserFormUI ui = userFormController.start(rm);
m.setScreen2Panel(ui.getMainPanel());
}

};
}

}





public class FormWizard {

private JFrame parentFrame;
protected UICenterSouthDialog screen1SelectReport;
protected UICenterSouthDialog screen2Dialog;
protected MultiColumnList screen1List;
protected PanelButtonWizard screen1Dialog;
protected PanelButtonWizard screen2Buttons;
private FormWizardModel m;

public FormWizard(JFrame frame, FormWizardModel m) {
super();
this.m = m;
this.parentFrame = frame;
}

public void start() throws Exception {
screen1SelectReport = new UICenterSouthDialog();
screen1SelectReport.setSize(m.getScreen1Size());
screen2Dialog = new UICenterSouthDialog();
buildScreen1(m.getScreen1Title(), m.getResultList());
buildScreen2(m.getScreen2Title(), m.getFinishButtonText());
addScreen1Listeners();
screen2Dialog.setSize(m.getScreen2Size());
addScreen2Listeners(m.getFinishAction());
screen1SelectReport.setVisible(true);
}

public void addScreen1Listeners() {

screen1List.addListSelectionListener(new ListSelectionListener()
{
public void valueChanged(ListSelectionEvent e) {
if (screen1List.isRowSelected()) {

screen1SelectReport.setUserObject(screen1List.getSelectedRow());
m.setUserObject(screen1List.getSelectedRow());
}

screen1Dialog.getCmdNext().setEnabled(screen1List.isRowSelected());
}
});
screen1Dialog.getCmdCancel().addActionListener(new
ActionListener() {
public void actionPerformed(ActionEvent e) {
screen1SelectReport.dispose();
}
});
screen1Dialog.getCmdNext().addActionListener(new ActionListener()
{
public void actionPerformed(ActionEvent e) {
try {
screen2Dialog.getCenterPanel().removeAll();
if (m.getNextAction() != null) {
m.getNextAction().actionPerformed(e);
}
screen2Dialog.getCenterPanel().add(m.getScreen2Panel(),
BorderLayout.CENTER);
screen2Dialog.setVisible(true);
screen1SelectReport.setVisible(false);
} catch (Exception ex) {
Debug.LogException(this, ex);
}
}
});
}

public void addScreen2Listeners(final ActionListener finishAction) {
screen2Buttons.getCmdCancel().addActionListener(new
ActionListener() {
public void actionPerformed(ActionEvent e) {
screen2Dialog.dispose();
}
});
screen2Buttons.getCmdBack().addActionListener(new
ActionListener() {
public void actionPerformed(ActionEvent e) {
screen1SelectReport.setVisible(true);
screen2Dialog.setVisible(false);
}
});
screen2Buttons.getCmdFinish().addActionListener(new
ActionListener() {
public void actionPerformed(ActionEvent e) {
try {
screen2Dialog.setVisible(false);
if (finishAction != null) {
finishAction.actionPerformed(e);
}
} catch (Exception ex) {
Debug.LogException(this, ex);
}
}
});
}

private void buildScreen1(String title, ResultList list) throws
Exception {
screen1SelectReport.setTitle(title);
// Report list
screen1List = new MultiColumnList();
screen1List.setTableModel(list, 80);
screen1SelectReport.getCenterPanel().add(screen1List,
BorderLayout.CENTER);
// Buttons
screen1Dialog = new PanelButtonWizard();
screen1Dialog.setButtonState(true, false,false, false);
screen1SelectReport.getSouthPanel().add(screen1Dialog,
BorderLayout.CENTER);
if (list.getRowCount() > 0) {
screen1List.selectRow(0);

screen1SelectReport.setUserObject(screen1List.getSelectedRow());
m.setUserObject(screen1List.getSelectedRow());
screen1Dialog.setButtonState(true, false,true, false);
}
}

private void buildScreen2(String screen2Title, String
finishButtonText) throws Exception {
// Controls
screen2Dialog.setTitle(screen2Title);
// Buttons
screen2Buttons = new PanelButtonWizard();
screen2Buttons.getCmdFinish().setText(finishButtonText);
screen2Buttons.setButtonState(true, true, false, true);
screen2Dialog.getSouthPanel().add(screen2Buttons,
BorderLayout.CENTER);
}
}

 

[Sergey Alpaev]

Hi,

I believe that the sympthom in the code of the problem mentioned below:

FormWizardModel which is populated at alternating times by either the
UserWizard or the FormWizard and that seems dangerous as it is
difficult using just code inspection to determine the state of the
FormWizardModel at any point in time.

is that FormWizardModel does not have any behaviour, just data. I would go
through the code and check for every case the data are requested from the
model if this is a candidate for method in the model or in this case the
data should be moved out of the model.
Few examples are following:
The code below gets Model member variable and makes a call on it:

if (m.getNextAction() != null) {

m.getNextAction().actionPerformed(e);

}

But value of the member variable is provided in UserWizard:
m.setNextAction(getNextAction());

I would move all action related staff back in to wizards keeping model free
from knowledge about actions. I would also keep model free from knowledge
about presentation things such as screen sizes. If these values are
retrieved from some external source (for example user preferences file) I
would put them into new class like WizardLayout which would keep all screen
configuration information. It would simplify model. Otherwise I would keep
them together in the classes which actually use them

Also, in my opinion, FormWizard tries to know too much of
UICenterSouthDialog internals. For example, it accesses its center panel,
south panel, internal controls (screen1List member variable, which is in
FormWizard but to me it looks like it is part of UICenterSouthDialog
visually on the screen). I would try to delegate more responsibilities to
UICenterSouthDialog, for example building itself (adding list, configuring
south panel, center panel, a lot of code from buildScreen1 method).

In general, the design would benefit from separation of responsibilities in
MVC style where the Controller would handle navigation, views would be the
UICenterSouthDialog-derived classes, which access shared model, I think.

Hope this helps.