Martins Design Study

Overview

My design study is about the reporting interface for my group (Team Patchwork's) Eclipse plugin which we designed last year. I wrote the reporting interface myself, but without using good OO design. Although it worked, it is a little bothersome to extend (I.E. to create new types of reports or new views an layouts). Hence, I decided to analyze and improve it somewhat.

Use Cases

The reporting interface would be used as follows:

• The user should be able to generate a report for any date range.
• The user should be able to generate a report for any metric defined by the plugin.
• The user should be able to edit an existing report, by changing the date range. The report display should update to reflect the new date range.
• The user should be able to add or remove metrics from an existing report. The report display should update to reflect the new date range.
• The user should be able to save a report as some sort of file (XML, CSV, anything will do, really).

Initial Design

A diagram of the initial design of the reporting interface is shown below.

Flaws

The flaws in the initial design include the following:

• There are some "support" classes which are not linked to any other classes. This surely must violate some design principles. TODO: Find these. The classes include: Support, ActivityReportTest, and ActivityReportAction.
• ActivityCategory is not linked to the report.
• The ActivityReportWizard class does not allow the containing of sub-forms. It is all packed into one huge form. This is inconvenient - it would be good to allow a wizard with multiple forms. It does allow for one page, but not many.
• The ActivityReportWizard class and ActivityReportWizardPage class both contain the ActivityReport object, making changes to it. This does not smell right!
• ActivityCategory is unused! This class is supposed to dictate which metrics appear in the report.
• ActivityCategory should perhaps be either a Composite or a Decorator. It is neither at the moment.
• There is no way to export to XML or any other file format. A new class should be introduced for this.
• The showChart() method of the ActivityReport class has a terrible Switch statement smell! Here are the details:

<syntaxhighlight lang="java">

public void showChart() {
       String title = "";
       if(chartType.equals("activity report"))
           title = (new StringBuilder(String.valueOf(title))).append("Activity Report for ").toString();
       if(chartType.equals("class activity"))
           title = (new StringBuilder(String.valueOf(title))).append("Class Report for ").toString();
       if(chartType.equals("view activity"))
           title = (new StringBuilder(String.valueOf(title))).append("View Report for ").toString();
       title = (new StringBuilder(String.valueOf(title))).append(System.getProperty("user.name")).append(" from ").append(DateFormat.getDateInstance().format(getStartDate())).append(" to ").append(DateFormat.getDateInstance().format(getEndDate())).toString();
       String windowTitle = "Patchwork Metrics report";
       String xAxisLabel = "Part";
       String yAxisLabel = "Time";
       DefaultCategoryDataset dataset = new DefaultCategoryDataset();
       GenericStorageObj storageLayerDataSet = null;
       if(chartType.equals("activity report"))
           storageLayerDataSet = createActivityChartData(getStartDate().getTime(), getEndDate().getTime());
       else
       if(chartType.equals("class activity"))
           storageLayerDataSet = createClassChartData(getStartDate().getTime(), getEndDate().getTime());
       else
       if(chartType.equals("view activity"))
           storageLayerDataSet = createViewChartData(getStartDate().getTime(), getEndDate().getTime());
       else
           return;
       String category;
       Integer timeSpent;
       for(Iterator iterator = storageLayerDataSet.getNestedObjects().iterator(); iterator.hasNext(); dataset.addValue(timeSpent, "Time Spent", category))
       {
           IStorableObj obj = (IStorableObj)iterator.next();
           GenericStorageObj tuple = (GenericStorageObj)obj;
           category = (String)tuple.getField("part").getValue();
           timeSpent = (Integer)tuple.getField("time").getValue();
       }
       org.jfree.chart.JFreeChart chart = ChartFactory.createBarChart(title, xAxisLabel, yAxisLabel, dataset, PlotOrientation.VERTICAL, false, true, false);
       ChartFrame frame = new ChartFrame(windowTitle, chart);
       frame.pack();
       frame.setVisible(true);

}</syntaxhighlight>

Note also how the JFreeChart chart is created. This is a good candidate for a Factory Method.

Improved Design

A diagram of the improved design of the reporting interface is shown below.

Changes Made

The following are the changes I made to better conform with OO Design principles.

• The "support" classes which were not communicating with other classes have been removed. Support and ActivityReportTest have both been removed from the package. ActivityReportTest should have been created in a separate "test" package. ActivityReportAction is still required, but has been left out of the diagram, as it relates to higher-level Eclipse Plugin code and is not really part of the core reporting module.

Patterns Used

In the improved version, I used the following design patterns:

• Added a set of ActivityCategory objects to the ActivityReport class to track the categories shown. Added appropriate methods to enforce the Law of Demeter.

• To enforce Separation of concerns, I created a GUI class to handle the View of the report. This would allow updating the report internals and then updating display as well. Before, the only time that SetStartDate() and getEndDate() were called was in the save() method of the ActivityReportWizardPage class (which meant that after creating the report, the dates could not be changed). TODO.

• The getActivityReportChartTypes() or the ActivityReport class creates and returns a set. Perhaps this should be made a static set or an Enum used instead. Perhaps it could even be abstracted into a Strategy. We can see there are 3 types of reports: activity, class, and view. At the moment, ActivityReport has three methods which operate in parallel: createActivityChartData(), createViewChartData(), and createClassChartData(). These should perhaps be absctracted into Factory Methods, and suggests that the Open closed principle is not being used. Anyone wanting to create a new type of report will have to edit the existing code. TODO.

• Perhaps different categories could be represented as a Decorator?

Discussion

The design project was to improve the reporting interface for the "Team Patchwork Reporting Application". TODO.