Martins Design Study

Revision as of 05:49, 2 September 2010

Overview

My design study is based on the "Patchwork Metrics" Eclipse plugin. This is a plugin for gathering "process metrics", such as time spent programming, time away from keyboard, amount of code written, and other useful metrics.

Patchwork Metrics runs in the background and collects metrics while the user is developing with Eclipse. From time to time, the user will want to see reports on the process metrics gathered by Patchwork Metrics. Therefore, a reporting interface was written to enable this. In this design study, I aim to redesign the reporting interface to fit the OO wisdom described in COSC 427.

I wrote the reporting interface myself, but without good OO wisdom in mind. The interface works, but is not very extensible. For instance, it would be difficult for other developers to to create new types of reports or new views and layouts without modifying the existing codebase. This violates to Open closed principle.

Categorization of Metrics

In Patchwork metrics, the metrics were divided into three main categories:

• Activity (which activity the developer is performing). This could include testing, developing, designing, and other activities.
• View (how much time the developer spends viewing different classes).
• Class (metrics specific to each class, such as "how often was the class edited?", or "what is the cyclomatic complexity of this class?").

This resulted, initially, in three different types of reports. There could be an "Activity Report", a "View Report", or a "Class Report". However, this was initially chunked together in one large class called "ActivityReport". It would be better for it to be broken up so that Separation of concerns is followed.

Use Cases

These are the use cases which drove the design changes I made.

Use Wizard to Create Report

The user can use a menu option or button to show a wizard (made of different pages) which allows them to create a report. There should be an option for creating any type of report mentioned above (in Categorization of Metrics).

• There is a menu option in Eclipse called "Patchwork Metrics Report".
• User clicks that button.
• A "Wizard" (http://en.wikipedia.org/wiki/Wizard_%28software%29) is shown. The user navigates through the wizard and chooses the type of report they want to generate and set its parameters.
• Once the user finishes the dialog, the results are displayed in a graphical format (a chart or graph).

The user can set all the necessary parameters for the report, including:

• Date range.
• Classes included in analysis.
• Developers included in analysis.

Extend to Create New Report Types

A third-party developer should be able to add a new report type (and a wizard for this report) without modifying the existing codebase (following the Open closed principle).

Export Report Results to XML Format

The results displayed in the report should be exportable to XML format. Ideally, this should be extensible enough so that a third-party developer can add a new type of file format for the report to be exported to. (Sounds like the Visitor pattern).

Edit existing report (from GUI)

Once the report has been displayed in graphical format, there should be controls which allow the parameters for the report to be changed. As these parameters are changed, the report details should be recalculated and the display updated. (Sounds like the Observer pattern).

Initial Design

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

Classes

This section describes the classes used in the design.

• ActivityReport: Contains the data for the report, as well as the parameters. This includes the starting and ending date for the report data, as well as the calcluated metrics for this date range.
• ActivityReportWizard: Represents the Wizard which the user goes through to create the report. This includes the pages and and navigation details for them. However, in the initial design it contained only one page.
• AcitivityReportWizardPage: This is one page of the wizard. The user will set the parameters using GUI-based controls (such as a calendar widget for setting the start and end dates). The settings made by the user should be reflected in the resulting report.
• ActivityCategory: Represents one category of an activity report. A category can be "Testing", "Developing", "Designing" or other activities. This sounds like a candidate for a strategy (a strategy for gathering and calculating metrics from the raw metrics data). A category can be divided into multiple sub-categories (however, this is not the case in the initial design).

Flaws

The flaws in the initial design include the following:

There are some "support" classes which are not linked to any other classes. The classes include: Support, and ActivityCategory. Support was not used at all in the final release last year, so it is debatable whether it should still be in the codebase. I decided to get rid of it for the improved design. ActivityCategory another concern, as it is not linked to the report. This is because we ran out of time to include categories in the reports before the final release.

The ActivityReportWizard class only contains one ActivityReportWizardPage. It should allow for a set or list of pages, and not just one. Again, we ran out of time to remedy this before the final release. In the improved design, I changed the field from a single page to a list of pages instead.

The ActivityReportWizard class and ActivityReportWizardPage class both have an association with (contain) the ActivityReport object, making changes to it. It could be argued that they are playing the role of a controller, but I believe the situation can be improved. There should be much looser coupling between the Wizard classes and the Report class.

An ActivityCategory should be able to consist of multiple sub-categories. For example, the category "testing" could consist of the sub-categories "writing tests" and "running tests". To enable this, in the improved version I changed ActivityCategory into a Composite. This relates to the use case mentioned above that a user should be able to "drill down" into the subcategories in an "Activity" report.

There is currently no mechanism for exporting the report data to XML.

The showChart() method of the ActivityReport class has a terrible Switch statement smell. See the "appendix" for details.

There are three methods in the ActivityReport class which do essentially the same thing. These are called "createXXXChart" where "XXX" is the report type. This suggests that Report should be an abstract class and that subclasses should override this type of method.

Improved Design

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

Changes Made

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.

I 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). The ReportGUI class is an Observer of the Report class.

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. In the improved design, this has been replaced with an abstract method createChartData() in the Report Class. The subclasses of Report now implement this method, getting rid of the Switch statement smell.

There are still more changes that could be considered. For example, 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". This was completed by using a variety of design patterns, as noted above. A few classes were renamed (such as "Activity Report" to "Report").

Conflicting Forces

There were some conflicting forces in the design.

• Simplicity of design vs. extensibility. This is visible especially with the Wizard adaptors / containers. There are two containers, one for each type Wizard GUI library. This is extensible, but it is hard to make both conform to a common interface. It is tempting to wrap calls to the specific API inside the container classes. For instance, if the JFace wizard had an API call named FaceScreen(), which the Netbeans Wizard API did not, then what should be done with the IWizard interface? Should we include the function, and implement a null override in NetBeansWizard? Or should we not include the method in IWizard, and sacrifice some cool functionality?

• Separation of concerns vs. convenience / simplicity : This is visible in the ReportGUI / Report relationship. ReportGUI is rather tightly coupled to Report, acting as sort-of a controller and observer at the same time. This could be more tightly coupled (convenience) or less tightly coupled (separation of concerns).

• Separation of Model and View: This is noticeable in the ReportGui / Report and Wizard / Report relationships.

Coming soon: The metrics for each design (old and new).

Appendix

The "showChart()" Method

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);
}