BrettWardsDesignStudy

Latest revision as of 03:08, 25 November 2010

For my design study I chose to work on the client part of my 325 group 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.

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 (mentioned above). 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

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. Also reduced the breach of Once and only once.
• 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 (which I forgot to show on the diagram, it seems, oops)

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.

Version 3

Started this version by splitting user and a new "plugin" class, which essentially does all the things that the plugin needs (Opens a ServerConnection, creates various bits, and acts as the go between for the start buttons on the Eclipse interface and the client code). While doing this I noticed that User, in it's initial implementation, was not really implemented. Obviously, we weren't sticking to things like Do the simplest thing that could possibly work and You ain't gonna need it last year.

Improvements

• User now acts as a proper entity of a user, rather than the horrible combination of things it was. Remainder of said horrible combination is now the Plugin class.
• Few no longer used fields, methods and whatnot were removed.

Other Stuff

• Not entirely sure what to do with the structure of the Debug* classes. Because they were designed by another group member using various Eclipse plugin development help websites, I'm tempted to leave them as they are, although the public field will be gone in the next version...
• Still need to mess with the way things interact with the Log. It's definitely a nicer design than it was.

Version 4 (probably the final version)

Improvements

• Observer added to Plugin to observe DebugTracker's changes. This allows the log to be updated without additional resources being passed around (ie, the Log). I only implemented this in this one place, but a similar approach could be used to decouple the Log from everything except the plugin, and thus allowing increased reusability of classes in other code (as, for the most part, classes know a lot less about other classes than they did originally).

Conclusions and Final Words

Future Work (and remaining problems)

As this didn't touch on the server-related parts of the program, those would need to be updated and tested. This would mostly be refactoring server-based methods and lines of code in similar ways to the non-server counterparts.

In this final version, AFKTracker still knows about Logs. This was not changed as it would've added other problems, like Avoid multiple inheritance (as AFKTracker currently extends Thread, and java will also not allow multiple inheritance to allow it to extend Observable too).

Conclusions

In this design study, a number of problems were found and remedied within the 325 project from last year, such as One responsibility rule, A class should not depend on its users and Beware singletons. Through a number of iterations these were fixed using patterns such as Observer.

From performing this study it has been quite clear how easy it is for simple rules, even something like You ain't gonna need it, is often ignored while creating software systems, and how multiple iterations can aid towards slowly finding a solution that is otherwise difficult to see.

Final files

The following file contains the final updated version of the design study. Updates were not made to the repository listed at the start of this page.

The full Eclipse plug-in (plus all those extras Eclipse adds) is included, and should easily be able to be imported into Eclipse as a plug-in project

File:ArtistMetrics.rar

Additional Notes

• UML diagrams created and/or imported via ArgoUML.
• The final project submission from last year was buggier than I remembered. I have not made an effort to change the implementation or add additional features that would fix or change these bugs.