Refactoring


In the course of building this example for the book, I coded the Sis constructor to include the statement that makes the frame visible. Michael Feathers reminded me that constructors are for initialization only. It's a bit of a stretch, I countered, but you could consider displaying the window as part of its initialization. To which he replied, using wisdom from Ron Jeffries, that a constructor has a lot of nerve doing something it isn't asked to do.

I listened to the voices of these two and fixed the deficiency. However, I still consider that GUI widget and layout construction is simple initialization. Putting panels within a frame is layout initialization. While you could conceivably separate this layout initialization from object initialization, there is little value in doing so. A JPanel subclass with uninitialized contents is of little use. Forcing clients to make an additional call to initialize its layout is redundant.

Separating the setVisible statement from the constructor is of potential value, however. In the case of SisTest, doing so allowed for a separate initialization test that doesn't force the frame to be rendered. In production systems, it's often valuable to initialize a frame behind the scenes, later controlling its visibility separately.

Ultimately the decision is somewhat arbitrary and thus academic. With regards to CoursesPanel, a future requirement may require a developer to create a CoursesPanel subclass. Putting initialization code in a separate method makes it easier to override or extend the initialization functionality. Yet you want to avoid designing for future what-ifs (that often never come). It's just as easy to wait and make the change down the roadif necessary.

An acceptable compromise is to consider simple design rule #3 and extract initialization to a private method for readability purposes:

 public CoursesPanel() {    createLayout(); } private void createLayout() {    JLabel label = new JLabel(LABEL_TEXT);    add(label); } 

The code in SisTest.containsCoursesPanel is very similar in form to that in -CoursesPanelTest.getLabel. Both methods loop through the components in a container and test for a match.

The duplicate code is a problem that will only get worse. Adding buttons, list boxes, or other component types will require a new method for each type. Additionally, suppose you have two labels on a panel. The current code will grab only the first label.

One solution is to provide a name for each component. You can then loop through the list of subcomponents until you find a matching name. This solution requires a little more management overhead in your view class, but it makes testing easier. It also can make aggregate operations on components easier, as you'll see later.

All component and container classes inherit from java.awt.Component. In this class you will find the methods setName and getName that respectively take and return a String object.

Starting with the code in SisTest:

 public void testCreate() {    final double tolerance = 0.05;    assertEquals(Sis.HEIGHT, frame.getSize().getHeight(), tolerance);    assertEquals(Sis.WIDTH, frame.getSize().getWidth(), tolerance);    assertEquals(JFrame.EXIT_ON_CLOSE,                 frame.getDefaultCloseOperation());    assertNotNull(getComponent(frame, CoursesPanel.NAME)); } private Component getComponent(JFrame frame, String name) {    Container container = frame.getContentPane();    for (Component component: container.getComponents())       if (name.equals(component.getName()))          return component;    return null; } 

The corresponding changes to CoursesPanel:

 package sis.ui; ... public class CoursesPanel extends JPanel {    static final String NAME = "coursesPanel";    ...    public CoursesPanel() {       setName(NAME);       createLayout();    } } 

Part of the secret to eliminating duplication is to push toward use of abstractions. For example, the new getComponent method deals in terms of abstract components instead of the concrete type CoursesPanel. Pushing the code in CoursesPanelTest in this direction leads to:

 package sis.ui; ... public class CoursesPanelTest extends TestCase {    public void testCreate() {       CoursesPanel panel = new CoursesPanel();       JLabel label =          (JLabel)getComponent(panel, CoursesPanel.LABEL_NAME);       assertEquals(CoursesPanel.LABEL_TEXT, label.getText());    }    private Component getComponent(Container container, String name) {       for (Component component: container.getComponents())          if (name.equals(component.getName()))       return component;    return null;    } } // in CoursesPanel: ... public class CoursesPanel extends JPanel {    static final String LABEL_NAME = "coursesLabel"; ...    private void createLayout() {       JLabel label = new JLabel(LABEL_TEXT);       label.setName(LABEL_NAME);       add(label);    } } 

At this point, the getComponent methods are different only by the first line. For lack of a better place, move them into a new class as class methods and refactor. Here is the code for the resulting class, sis.ui.Util, as well as for the modified test classes:

 // sis.ui.Util package sis.ui; import java.awt.*; import javax.swing.*; class Util {    static Component getComponent(Container container, String name) {       for (Component component: container.getComponents())          if (name.equals(component.getName()))             return component;       return null;    }    static Component getComponent(JFrame frame, String name) {       return getComponent(frame.getContentPane(), name);    } } // sis.ui.SisTest public void testCreate() {    final double tolerance = 0.05;    assertEquals(Sis.HEIGHT, frame.getSize().getHeight(), tolerance);    assertEquals(Sis.WIDTH, frame.getSize().getWidth(), tolerance);    assertEquals(JFrame.EXIT_ON_CLOSE,                 frame.getDefaultCloseOperation());    assertNotNull(Util.getComponent(frame, CoursesPanel.NAME)); } // sis.ui.CoursesPanelTest public void testCreate() {    CoursesPanel panel = new CoursesPanel();    JLabel label =       (JLabel)Util.getComponent(panel, CoursesPanel.LABEL_NAME);    assertEquals(CoursesPanel.LABEL_TEXT, label.getText()); } 

The new method calls are a bit more unwieldy. One reason is the new need to cast. You'll refactor when or if casting duplication becomes apparent. But the short-term solution is a vast improvementyou've eliminated method-level duplication that otherwise would become rampant in short time.



Agile Java. Crafting Code with Test-Driven Development
Agile Javaв„ў: Crafting Code with Test-Driven Development
ISBN: 0131482394
EAN: 2147483647
Year: 2003
Pages: 391
Authors: Jeff Langr

flylib.com © 2008-2017.
If you may any questions please contact us: flylib@qtcs.net