Part 1: Original Code Code (Online at www.xp123.com/rwb )
Table.java import java.awt.*; import java.awt.event.*; import javax.swing.*; import javax.swing.event.*; import javax.swing.text.JTextComponent; public class Table extends JFrame { public static void main(String[] args) { new Table(); } private JComponent body; private Card selection; private JLabel summary; int velocity = 1; int budget = 4; JTabbedPane tabs; public Table() { super("Planning Game Simulation"); JComponent buttons = makeButtons(); this.getContentPane().add(buttons, "West"); body = new Background(1000, 1000, 190); body.addContainerListener(new ContainerListener() { // Listen for container changes so we know when // to update selection highlight public void componentAdded(ContainerEvent e) { resetSelection(); } public void componentRemoved(ContainerEvent e) { resetSelection(); } private void resetSelection() { if (selection != null) selection.setBorder( BorderFactory.createLineBorder(Color.blue, 6)); if (body.getComponentCount() == 0) { selection = null; } else { selection = (Card) body.getComponent(0); selection.setBorder( BorderFactory.createLineBorder(Color.red, 6)); } } }); JScrollPane scroll = new JScrollPane(body); scroll.setPreferredSize(new Dimension(100, 100)); this.getContentPane().add(scroll, "Center"); summary = new JLabel("", SwingConstants.CENTER); summary.setText(summary()); this.getContentPane().add(summary, "South"); this.pack(); this.setSize(800, 600); this.show(); } private JComponent makeButtons() { JPanel panel = new JPanel(new GridLayout(0, 1)); panel.add(new JLabel("Customer")); makeCustomerButtons(panel); panel.add(new JLabel(" ")); panel.add(new JLabel("Programmer")); makeProgrammerButtons(panel); JPanel outer = new JPanel(new BorderLayout()); outer.add(panel, "North"); outer.add(new JLabel(""), "Center"); return outer; } private void makeCustomerButtons(JPanel panel) { JButton button; button = new JButton("New"); panel.add(button); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { Card card = new Card(); body.add(card, 0); selection = card; updateCost(); repaint(); } }); button = new JButton("Split"); panel.add(button); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { if (selection == null) return; Card card = new Card(selection); body.add(card, 0); selection = card; updateCost(); body.repaint(); } }); button = new JButton("Delete"); panel.add(button); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { if (body.getComponentCount() == 0) return; body.remove(0); selection = null; if (body.getComponentCount() != 0) selection = (Card) body.getComponent(0); updateCost(); body.repaint(); } }); button = new JButton("Plan"); panel.add(button); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { StringBuffer report = new StringBuffer(); // Check for cards that need est. or splitting for (int i = 0; i < body.getComponentCount(); i++) { Card card = (Card) body.getComponent(i); if (card.needsEstimate()) report.append( "Needs estimate: " + card.title() + "\n"); else if (card.needsSplit()) report.append( "Needs to be split: " + card.title() + "\n"); } if (report.length() == 0) JOptionPane.showMessageDialog( body, "Plan OK; no cards need estimates or splitting", "Issues in plan", JOptionPane.OK_OPTION); else JOptionPane.showMessageDialog( body, report.toString(), "Issues in plan", JOptionPane.OK_OPTION); } }); } private void makeProgrammerButtons(JPanel panel) { JButton button; button = new JButton("Cost"); panel.add(button); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { if (selection == null) return; selection.rotateCost(); updateCost(); } }); button = new JButton("Velocity"); panel.add(button); button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { velocity++; if (velocity >= 10) velocity = 1; updateCost(); } }); } private void updateCost() { summary.setText(summary()); } private String summary() { StringBuffer result = new StringBuffer(); result.append( "Est. Velocity (points/iteration): " + velocity + ". "); result.append("Total cost (points): " + cost() + ". "); result.append("#Cards: " + body.getComponentCount() + ". "); result.append( "Est. #iterations: " + (cost() + velocity - 1)/ velocity + ". "); return result.toString(); } // Total cost of the set of cards private int cost() { int total = 0; for (int i = 0; i < body.getComponentCount(); i++) { Card card = (Card) body.getComponent(i); total += card.cost(); } return total; } } Background.java import java.awt.*; import javax.swing.*; public class Background extends JLayeredPane { int lineDistance; public Background(int width, int height, int lineDistance) { super(); setPreferredSize(new Dimension(width, height)); this.lineDistance = lineDistance; setOpaque(true); setBackground(Color.yellow); setForeground(Color.orange); } public void paintComponent(Graphics g) { super.paintComponent(g); int height = getHeight(); for (int i = 0; i < this.getWidth(); i+= lineDistance) g.drawLine(i, 0, i, height); } } Card.java import java.awt.*; import java.awt.event.*; import javax.swing.*; import javax.swing.border.*; public class Card extends JPanel { static int cards = 0; final static int scale = 34; // card size factor final static int height = 3 * scale; // 3x5 final static int width = 5 * scale; final static int rollover = 12; // New cards go in difft. pos'ns final static int offset = scale;// How far apart to put new cards JTextField title; JTextArea body; JLabel costLabel; int cost; public Card() { this("(Untitled)", ""); } public Card(Card from) { this(from.title.getText(), from.body.getText()); } private Card(String titleText, String bodyText) { super(); setBorder(BorderFactory.createLineBorder(Color.blue, 6)); cards++; setLayout(new BorderLayout()); JPanel top = new JPanel(new BorderLayout()); title = new JTextField(titleText); title.setBackground(Color.pink); title.addFocusListener(new FocusAdapter() { public void focusGained(FocusEvent e) { moveToFront(); } }); top.add(title, "Center"); costLabel = new JLabel(" ? "); cost = 0; top.add(costLabel, "East"); add(top, "North"); body = new JTextArea(bodyText); body.setLineWrap(true); body.setWrapStyleWord(true); body.setBackground(Color.pink); body.addFocusListener(new FocusAdapter() { public void focusGained(FocusEvent e) { moveToFront(); } }); add(body, "Center"); setLocation( (cards % rollover) * offset, (cards % rollover) * offset); setSize(width, height); // Turn on mouse events so we can detect being dragged enableEvents( AWTEvent.MOUSE_EVENT_MASK AWTEvent.MOUSE_MOTION_EVENT_MASK); } private void moveToFront() { JLayeredPane background = (JLayeredPane) this.getParent(); background.moveToFront(this); } private int lastx, lasty; public void processMouseEvent(MouseEvent e) { if (e.getID() == MouseEvent.MOUSE_PRESSED) { lastx = e.getX(); lasty = e.getY(); moveToFront(); } else super.processMouseEvent(e); } public void processMouseMotionEvent(MouseEvent e) { if (e.getID() == MouseEvent.MOUSE_DRAGGED) { Point here = getLocation(); setLocation((int) (here.getX() + e.getX() - lastx), (int) (here.getY() + e.getY() - lasty)); } else super.processMouseMotionEvent(e); } public void rotateCost() { String label = costLabel.getText(); if (label.equals(" ? ")) { costLabel.setText(" 1 "); cost = 1; } else if (label.equals(" 1 ")) { costLabel.setText(" 2 "); cost = 2; } else if (label.equals(" 2 ")) { costLabel.setText(" 3 "); cost = 3; } else if (label.equals(" 3 ")) { costLabel.setText(" >3 "); cost = 0; } else if (label.equals(" >3 ")) { costLabel.setText(" ? "); cost = 0; } else { // shouldn't happen costLabel.setText(" ? "); cost = 0; } } public int cost() { return cost; } public String title() { return title.getText(); } public boolean needsSplit() { return costLabel.getText().equals(" >3 "); } public boolean needsEstimate() { return costLabel.getText().equals(" ? "); } } Challenges | Exercise 91 Smells. (Challenging). | -
What smells do you sense in this code? Use the list in the Fowler's Refactoring catalog for inspiration; you'll probably find other things as well. -
What strategy (order of refactorings) would you use to improve the code?  See Appendix A for solutions. | | Exercise 92 Tests. (Challenging). | What about tests? The author of this code claimed, "It's basically all GUI , so I couldn't test it." Identify two or three strategies by which we could test this code, with little alteration to the current structure. See Appendix A for solutions. See Appendix A for solutions. | | Exercise 93 Write Tests. | Write at least one test for each class, using the simplest strategy you identified. See Appendix A for solutions. |
When faced with a moderately big refactoring task like this, I like to start easy by first fixing smells like Poor Name or Long Method. When code has many things that can be improved, it can be helpful to work with it on its own terms for a while before trying dramatic improvements. | Exercise 94 Dead Code. | There is unused code in the Table class (in the form of unreferenced variables ). Remove the dead code. See Appendix A for solutions. | | Exercise 95 From Table to PlanningGame. | The class name Table stinks as the name for the top-level class. (What table?!?) Even Main would be a better class name; it wouldn't communicate much, but at least it wouldn't mis communicate. Let's try PlanningGame as the name for this class: Rename the class appropriately. | | Exercise 96 Anonymous Inner Classes. | The PlanningGame class has a number of places that use an anonymous inner class to set up event listeners. (Anonymous inner classes are the ones where the routines are defined in the middle of code; you can spot them by seeing }); somewhere.) -
Convert Anonymous Inner Class to Inner Class by extracting each anonymous inner class to its own inner class. -
Would they be even better as standalone classes? See Appendix A for solutions. | | Exercise 97 Magic Numbers . | There are many magic numbers floating around. (Some of them might be more accurately called magic colors .) Extract each to the top of the class as a final static variable. (Focus on numbers and colors; we'll deal with strings later.) See Appendix A for solutions. |
 |
