Ritu Agrawal wrote:
> Hi everyone,
>
> Our text based wizards currently have a run-time dependency on JATO.
> JATO is not open sourced so, for support on OpenSolaris we need to
> remove this dependency.
>
> The JATO class that we depended on was DefaultModel.java and I have
> removed that dependency by adding two new classes - CliDefaultModel and
> CliDSConfigWizardModel. The GUI wizards still use the old classes. These
> changes touch all the text-based wizards.
>
> The changes can be reviewed at http://cr.opensolaris.org/~ritua/dswizards.
>
Ritu,
With the caveat that I've never looked at this code before, here are my
comments.
This code (from
usr/src/lib/dsconfig/classes/com/sun/cluster/dswizards/apache/ApacheWizardFlowManager.java
) seems like an indication that subtyping and polymorphism aren't being
used effectively.
57 public String getNextPanel(String curPanelName, Object wizCxtObj,
58 String options[]) {
59 DSConfigWizardModel guiModel = null;
60 CliDSConfigWizardModel cliModel = null;
61
62 if (wizCxtObj instanceof CliDSConfigWizardModel) {
63 cliModel = (CliDSConfigWizardModel)wizCxtObj;
64 } else if (wizCxtObj instanceof DSConfigWizardModel) {
65 guiModel = (DSConfigWizardModel)wizCxtObj;
66 }
67
68 // if current panel is docRootPanel
69 if (curPanelName.equals(ApacheWizardConstants.DOCROOTPANEL)) {
70
71 Boolean displayFSTable = null;
72 if (guiModel != null) {
73 displayFSTable = (Boolean)guiModel.getWizardValue(
74 ApacheWizardConstants.DISPLAY_FS_TABLE);
75 } else {
76 displayFSTable = (Boolean)cliModel.getWizardValue(
77 ApacheWizardConstants.DISPLAY_FS_TABLE);
78 }
Ideally, we would have a generic super-type of both
CliDSConfigWizardModel and DSConfigWizardModel (which is really the GUI
one). So I would rename DSConfigWizardModel to GUIDSConfigWizardModel
and have both that one and the CLI one be subclasses of a generic
DSConfigWizardModel. DSConfigWizardModel would declare a
getWizardValue() method, and each of the subclasses would override it as
appropriate.
Then the code above could be simplified to:
57 public String getNextPanel(String curPanelName,
DSConfigWizardModel wizCxtObj,
58 String options[]) {
67
68 // if current panel is docRootPanel
69 if (curPanelName.equals(ApacheWizardConstants.DOCROOTPANEL)) {
70
71 Boolean displayFSTable = null;
73 displayFSTable = (Boolean)wizCxtObj.getWizardValue(
74 ApacheWizardConstants.DISPLAY_FS_TABLE);
As a side benefit, most of the changes you made to refer to
CLIDSConfigWizardModel instead of DSConfigWizardModel wouldn't be needed
(though perhaps you'd need other additional changes).
You wouldn't then need to use the generic Object, and could use the more
specific DSConfigWizardModel when the actual object could be either the
CLI or GUI class.
Also, the code in CLIDSConfigWizardModel and DSConfigWizardModel looks
awfully similar. By subclassing them both from a common parent, you
could unify the shared code instead of repeating it in both classes.
(I realize you've got the DefaultModel class in the hierarchy as well --
perhaps the split needs to be at that level -- I'm not familiar enough
with the code to say)
Perhaps I'm missing some obvious reason about my proposed kind of
hierarchy wouldn't work in this specific situation, but generically it
seems like the right thing to do.
Thanks,
Nick
