BrettWardsDesignStudy

Revision as of 01:27, 4 October 2009

For my design study I chose to work on the client part of my 324 project last year. The topic was a metrics collection plugin for Eclipse, and my groups design used a server + client design, but the client "design" was somewhat hacked together.

As the client was an Eclipse plugin, a number of the classes are generated by Eclipse. These have mostly been left out of the UML diagrams, as they will remain unchanged among versions.

The final version from last year can be downloaded from Google Code, with the eclipse-plugin folder being the client I was working on.

Intro

The aim of the project was to create an Eclipse plug-in that took in data from various usage metrics, and then allowed users to see and access the data in a number of ways. The client-side, however, just dealt with collecting the metrics, and sending them to the server (as well as dumping it to a local log, for usage when not connected to a server).

At the end of the year, several data points could be collected from the plug-in, including:

• What programs had been in focus over a session, and for how long (aka, a focus tracker)
• What time (if any) a user had been away from their keyboard.
• Debugger usage tracking - including launch times, and markers on where breakpoints were put.
• Ratings metrics via a thumbs up/thumbs down view, allowing a user to rate a class up or down depending on their opinion.
• Generic messaging, allowing a user to comment on code or whatnot anonymously.

The Eclipse Plugin

The client plug-in consisted of two packages: actions and views. Actions consisted of 12 classes, and 3 views were created. Other classes exist within the Googlecode repository, but were from earlier iterations (in some cases classes were renamed) and thus have not been included in this study.

Focus is on the Actions part of the interface, although mention of connections to the Views parts will probably be brought up.

Analysis of Current Code

The UML diagram shows roughly where the design was at at the end of last year. Note some of the connections in the diagram may be slightly incorrect.

- diagram 1 goes here -

Notes on the initial design:

• The User class was somewhat the central class of the design, controlling a lot of the functionality whether it be metrics, or server-oriented. It was instantiated by Eclipse when either MetricsActions buttons were activated (which could only be done once).
• The MetricsActions classes were the classes instantiated by Eclipse when the plugin was activated. These were activated by buttons on the interface. Because of how Eclipse did this, we never found a way to make them one class with 2 different constructors.
• FocusTracker was a singleton class that kept track of all focus changes over a session, each change having it's own Timer. These were kept in a Map that contained the name of the item (eg, java class) brought into focus, and an arraylist of Timers related to items by that name.
• The User had a Log, which outputted all data to a text file, as well as the User passing the information to the LogChangesGUI if it was open

Patterns, Maxims and other features Found

• Singleton: While many classes are only instantiated once in last year's version, only User really fits the Singleton pattern.
• Hide Data Within Its Class: For the most part, this maxim has been followed. There are a few fields breaking this, which can easily be fixed.
• Model the Real World: Also relatively well followed, other than a few ewws below.
• Methods all seem within reasonable lengths, so no need to apply "Reduce the size of methods". Was somewhat surprised by this.

Possibly "Ewww" stuff

• Beware Singletons: A number of the classes in this design only have one instance (User, FocusTracker, etc). This could be worth looking into
• One Responsibility Rule: User has multiple responsibilities: It models a user, and it deals with various data flow. The "model the real world" conflict does not seem to be broken by this.
• Getters and Setters: Although "Hide Data Within Its Class" is generally followed, there is heavy use of getters and setters. Might be worth checking if all of these are worthwhile. or even used in the current version
• A class should not depend on its users: AFKTracker and DebugTracker know that they are being used by a User, and keep that User within their fields. This harms potential reusability of the classes.
• AFKTracker writes to Log via User. Should really not depend on Log, but given the design intention it'd be "nicer" to depend on a log rather than a user.
• DebugTracker has the User public, which the debug listeners use.
• ...a lot of classes in general depend on User far too much. I had not noticed this until now....

Ideas/Thoughts from Analysis

• The current implementation of FocusTracker seems to be breaking a few rules. It's somewhat more a "FocusTrackers" or "FocusTrackerControl" class, rather than a singular FocusTracker.
• User knows far too much. While it does contain an actual User, it also contains information on how to be a FocusTracker (via selectionChanged), and a Log (via writing to the LogChangesGUI updates, which is also a tad DRY, as there are two similar updates to different places in completely different classes). Concerns probably need to be seperated and tied to their correct classes.
• AFKTracker and Debugtracker hold the User they are tied to, whilst FocusTracker, Log and serverConnection are held in User.

Updated Version

--Diagram goes here--

Notes on improvements

• FocusTracker got cut down a lot, and now only performs basic Tracker controls (and could probably be cut town to the extent it's just a tracker). It is no longer an entity that is all FocusTrackers, but is now a singular instance. User currently creates FocusTrackers based on changed selections.
• Log now controls the GUI relating to the log. Still not a perfect option, but this lowers the amount of dependencies on other classes.
• DebugTracker and AFKTracker now have no need to know what a user is, instead they know how to use the log.
• writeToLog removed from User.

Issues with this version

• The connections between views and actions could be different, maybe? LogchangesGUI and Log being together makes more sense, and removes the problem with user knowing how to be a log in relation to updating the GUI, but still seems messy
• User knows even more now, in some aspects.
• DebugTracker and it's listener's still have awkward Log usage dependencies.
• User is still a "User" and a "Thing that deals with Eclipse Plugin functions". The next step will be refactoring into 2 classes due to these two roles.

Notes

• UML diagrams currently not uploaded