James Ashford Design Study
m (→Design Patterns Used) |
m (→Design Patterns Used) |
||
| Line 308: | Line 308: | ||
* ConcreteObserver = BreakpointEventDataStore | * ConcreteObserver = BreakpointEventDataStore | ||
| − | There is one slight adjustment to the standard Observer pattern - the ConcreteObserver is told about new data (rather than them querying the ConcreteSubject) - see [[Tell don't ask]]. This was done to avoid the ConcreteSubject recording the state information, and instead notifying subjects of new information. It also means that we can easily multi-thread the application without worrying about clients missing data. | + | There is one slight adjustment to the standard Observer pattern - the ConcreteObserver is told about new data (rather than them querying the ConcreteSubject) - see [[Tell don't ask]]. This was done to avoid the ConcreteSubject recording the state information, and instead notifying subjects of new directly information. It also means that we can easily multi-thread the application without worrying about clients missing data. |
The current design only has a single observer (a simple data store to record all the breakpoints because visualisations are generated after runtime). You can imagine that it would be easy to implement another visualisation to be generated during runtime using the current Observer pattern. | The current design only has a single observer (a simple data store to record all the breakpoints because visualisations are generated after runtime). You can imagine that it would be easy to implement another visualisation to be generated during runtime using the current Observer pattern. | ||
Revision as of 07:49, 28 September 2010
Contents |
My Project
As part of my honours project, I am developing a plug-in to Eclipse to help illustrate how the state of software changes during runtime. My design study will focus of the data collection subsystem of my project.
Data collection works by inserting a data collection point (also known as a Pseudo Breakpoint) into a line of code in your document. Every time the application hits this data collection point, a copy of all variables in scope (local, instance and static variables) are recorded and the program resumes. At the conclusion of runtime, the user can review what happened and generate visualisations.
There are two main areas in the data collection subsystem namely:
1. Management (inserting / deleting of data collection points etc)
2. Event Handler (whenever the data collection point is encountered)
Management
The management area of this project is related to the creation, deletion, insertion and management of the data collection points. These data collection points are known as Pseudo Breakpoints within the project. A data collection point is a thinly disguised breakpoint which is handled along side normal Java breakpoints.
Main activities:
- Creates Pseudo Breakpoints
- Manages the Pseudo Breakpoint
- Adds the extension to the Eclipse editor (so the user can toggle the Pseudo Breakpoints on/off
Event Handler
The event handler controls all the data collection events within the system.
Main activities:
- Handles events when the Pseudo Breakpoint is 'hit'
- Variable datastore (when the breakpoint is hit a copy of all inscope variables are stored in a datastore)
Event Handler Process Flow:
1. Event Handler is notified that a breakpoint has been hit
2. Event Handler checks to see if it was a PseudoBreakpoint
3. If so, do data collection
4. Event Handler checks to see if there was another breakpoint also hit (a normal breakpoint)
5. If so, suspend application (so normal breakpoint activity can occur)
6. Else resume program
Design Study
Requirements
- Maintainability - Easily add new features (such as different data sources etc)
- Extensible - Add additional programming languages (such as PHP etc) easily
Constraints
As this is an Eclipse plug-in, our design must fit the Eclipse model.
Initial Design
One Word: horrible.
UML Diagram
Description of Classes
debugassist:
- Activator (extends AbtractUIPlugin) - Activator class which is run when the Plugin is loaded
- DebugAssistLogic (implements DataUpdateBreakpointClient, DataUpdateEventClient) - Provides some basic functions to clean up breakpoint data, contains 2 clients which are activated when a breakpoint is inserted/updated/remove and when a breakpoint is 'hit'
debugassist.datasource:
- DataBreakpointStore - Stores all the pseudo breakpoints in a HashSet.
- DataEventStoreTime - Store all the data when a breakpoint is hit.
debugassit.events:
- DataUpdateBreakpointClient - An interface which needs to be implemented if you wish to listen to Breakpoint Events (i.e. when a breakpoint is added/removed etc)
- DataUpdateBreakpointListener - The service which handles Breakpoint listener clients (where clients subscribe to events etc)
- DataUpdateEventClient - An interface which needs to be implemented if you wish to listen to Breakpoint Data Events (i.e. when a breakpoint is encountered by the debugger).
- DataUpdateEventListener - The service which handles Breakpoint data listener clients (where clients subscribe to events)
debugassist.handlers:
- BreakpointAddRemoveEventHandler (implements IBreakpointListener) - handles the breakpoint events from Eclipse (i.e. when a breakpoint is added / removed from Eclipse) - and notifies DataUpdateBreakpointListener.
- DebugEventHandler (implements IDebugEventSetListener) - Handles the breakpoint hit events from Eclipse (i.e. when a breakpoint is hit within Eclipse) - this performs data collection (collects all variables in scope), and notifies DataUpdateEventListener.
- EclipseDebuggerEventListenerManager - Attaches BreakpointAddRemoveEventHandler and DebugEventHandler to Eclipse.
debugassist.model:
- BreakpointEvent - A model for whenever a breakpoint is hit by the debugger (stores time, variables)
- ModelGenerator - A utility class for converting IVariable (Eclipse type) to Variable (our model type)
- Variable - A model for a Variable
debugassist.pseudobreakpoint
- AddRemovePseudoBreakPointHandler (extends AbstractRulerActionDelegate) - Pseudobreakpoint handler type - so users can right click in Eclipse to add/remove a breakpoint.
- PseudoBreakpointImpl (extends JavaLineBreakpoint) - The breakpoint used to represent our Pseudobreakpoint. It is actually just a JavaBreakpoint with a different type and icon so the DebugEventHandler knows which breakpoints to collect.
Inherited Classes
A few classes are extended and implemented in this project
AbstractUIPlugin - An abstract class providing some basic functionality required to create an plugin in Eclipse. AbstractUIPlugin Javadoc
IBreakpointListener - An interface used by classes that require notification when breakpoints are added and removed IBreakpointListener Javadoc
IDebugEventSetListener - An interface used by classes that require notification when Debugger Events occur IDebugEventSetListener Javadoc
AbstractRulerActionDelegate - An abstract class to allow contributions to the vertical ruler's context menu in the text editor. AbstractRulerActionDelegate Javadoc
JavaLineBreakpoint - A class which provides the Java Line Breakpoint Implementation (required to insert a breakpoint into the JVM)
Design Critique
Initial Design
Quick notes:
- The current package design is inconsistant with poor names.
- DebugAssistLogic doesn't seem to do much, needs a reorg - most functions could be placed in Activator
- DebugEventHandler handles the Breakpoint data events. This is a clear violation of [Separation of concerns] - which it really should be a DataUpdateEventListener
- DataUpdateEventListener & DataUpdateBreakpointListener SHOULD be the ones themselves which handle the Eclipse events (the debugassist.handlers are unnecessary)
- The names of the classes need to be changed
- ModelGenerator should be fixed (I'm not sure I actually need it???)
- The Pseudobreakpoint classes should be fixed up - It's directly extending the Javabreakpoint class!!!!
- Many classes are singleton - perhaps this should be reorganised?
- It currently is hard-coded to support JavaBreakpoint only - make it more extensible!
There was no initial design - it all evolved out of prototyping - which is why the DebugEventHandler handles the breakpoint events.
Maxims Violated
Avoid inheritance for implementation My PseudoBreakpoint class directly inherits the Javabreakpoint class so that it can interact with the JVM. This effectively stops any other programming language from using our plugin.
[Separation of concerns] DebugEventHandler handles the Breakpoint data events as well as being an Event Listener.
Beware singletons There is a number of singleton classes in here including: ... .. .... Perhaps this could be refactored into a single class?
Single responsibility principle PsuedoBreakpoint class doing too much.
Program to the interface not the implementation Everything uses the direct class (no interfaces used) - Again, this limits our project to Java.
Software reuse DataUpdateBreakpointListener and DataUpdateEventListener both have the same code to handle the adding and removal of listener classes.
Model the real world Not really.
First Attempt Design Improvements
Things seem more organised now:
- Class names have become clearer (PsuedoBreakpointRulerActionDelegate & PseudoBreakpointRulerAction should be clearer than AddRemovePsuedoBreakpointHandler & PseudoBreakpointImpl)
- PseudoBreakpoint Factory
- Events Improved
- Model Generator & Handlers removed
PseudoBreakpoint Factory
Previously the PseudoBreakpoint class was a extension of the JavaBreakpoint class. In order to support other programming languages (in the future) I created a Factory Method to return the appropriate Breakpoint class to insert into the virtual machine.
- PseudoBreakpoint (extends IBreakpoint) - A placeholder type for our PseudoBreakpoint
- PseudoBreakpointCreator - Determines which sort of Breakpoint to create and returns the generic PseudoBreakpoint
- PseudoBreakpointPython / PseudoBreakpointJava (implements PseudoBreakpoint) - an extension of the Java/Python breakpoint class.
Events (BreakpointDataEvent / BreakpointChangeEvent)
Observer - My original design was kind-of an Observer pattern (but poorly implemented - both classes had the same code for adding/removing observers (Don't repeat yourself)). As Java's Observer pattern does not suport generics, I decided to create my own. A generic abstract class 'Subject' handles subscribers.
- Subject - Abstract class to handle observers attaching/detaching to the event handler
- DataUpdateEventObserver (was DataUpdateEventClient) - Essentially the same, but now with a proper name
- DataUpdateBreakpointObserver (was DataUpdateBreakpointClient) - Essentially the same, but now with a proper name
- BreakpointDataEvent [extends Subject, Implements IDebugEventSetListener (Eclipse)] (was DataUpdateEventListener) - Handles Breakpoint Data Events (i.e. when a breakpoint is 'hit') from the Eclipse Debugger directly and notifies subscribed observers
- BreakpointChangeEvent [extends Subject, Implements IBreakpointListener (Eclipse)] (was DataUpdateEventListener) - Handles Breakpoint Add/Remove Events from the Eclipse directly and notifies subscribes observers and the DataBreakpointStore (I think I need to modify this)
Variables / BreakpointEvent
I removed the model generator - it is now all handled within the BreakpointEvent class.
Third Attempt
Quick notes:
- Redesigned variable section (major changes here)
- Abstract class Variable (contains most of a variable definition)
- PrimitiveVariable class (contains data about primitive variables)
- ObjectVariable class (contains data about objects)
- Fixed PseudoBreakpointRulerActionDelegate classes - now using a subclass instead of the interface
- Removed the last Singleton pattern from BreakpointEventDataStore
- Turns out that it wasn't even necessary
- It is now all handled by the Activator class (which handles the creation of the plugin)
- Removed DataBreakpointStore. We didn't really need it - as we can query the underlying model if we ever need it.
- Slight rename of classes...
- BreakpointEventData is now called BreakpointDataEvent (the same as BreakpointDataEventStore)
- Collections implementations are now hidden (most generic type possible0
- Collections are now returns as unmodifiable (where applicable)
Packages
| Package | Description |
|---|---|
| debugassist | The root package. Only contains a single class (Activator). |
| debugassist.datasource | Contains the data store for all the event data |
| debugassist.events | Encapsulates all the Event handling classes (from the Eclipse model) |
| debugassist.model | Our custom model (Variables and BreakpointEvents) |
| debugassist.pseudobreakpoint | Classes to handle and represent the pseudobreakpoint in the text editor and in the associated virtual machines.
|
Class Descriptions
| Package | Class Name | Description | Public Attributes | Public Methods |
|---|---|---|---|---|
| debugassist (root) | class Activator (Extends AbstractUIPlugin) | This class launches the entire plugin. It is instantiated by Eclipse itself, and operates in a similar way to a singleton. It contains the logic to attach all the event handlers to Eclipse. | None | class Activator() - public constructor (required by Eclipse); start(BundleContext context) - starts and launches the plugin (and attaches event handlers); stop(BundleContext context) - destroys instance, removes event handlers; static Activator getDefault() - Returns the current instance of the running plugin; static ImageDescriptor getImageDescriptor(String path) - Returns the image descriptor for the image file. |
| debugassist.datasource | BreakpointEventDataStore (implements DataUpdateEventObserver) | This class stores the of BreakpointEvents as they occur. | None | ArrayList<BreakpointEvent> getAllBreakpointEvents() - returns the data store; clearEvents() - removes all items from the data store; updateModelRequired(String frameName, String threadName, IVariable[] variables) - A method from the DataUpdateEventObserver interface - it is called whenever a Pseudobreakpoint is hit. |
| debugassist.events | abstract class Subject<T> | Provides basic functionality to add and remove observers. | None | attach(T child) - adds a child who requires notification when something changes; detach(T child) - removes the child from notification; |
| debugassist.events | interface DataUpdateEventObserver | Any class which wishes to be notified whenever a Pseudobreakpoint is hit must implement this interface. | None | updateModelRequired(String frameName, String threadName, IVariable[] variables) - Whenever a Pseudobreakpoint is hit, this method will be called - containing the frame name, thread name, and an array of IVariables (An Eclipse model type) |
| debugassist.events | class BreakpointEventData extends Subject<DataUpdateEventObserver> implements IDebugEventSetListener | Handles all Eclipse breakpoint data events (like when a breakpoint is hit by the debugger), and passes relevant information (only pseudobreakpoint info) onto the observers. | None | handleDebugEvents(DebugEvent[] events) - Required by IDebugEventSetListener interface, is called whenever any breakpoint is hit in Eclipse. Our implementation determines if the breakpoint was a pseudo breakpoint, and if so, notifies all the attached observers. dispose() - basic dispose operation to remove all children from collection. |
| debugassist.model | abstract class Variable implements Comparable<Variable> | Provides an abstract presentation of a variable. There is also a private class ModelGenerator which converts an IVariable array (the Eclipse type) to our Variable array. | None | String getData() - returns a String representation of the data; String getType() - returns a String of the type; String getName() - returns a String of the name of the variable. getUpdateCount() - returns the number of changes that variable has had; updateData(String data) - updates the data field of the variable (and increments the update counter); compareTo(Variable o) - compares another variable against itself for sorting (based on the name of the variable). |
| debugassist.model | class PrimitiveVariable extends Variable | Represents a primitive variable (eg int) | None | boolean isLeaf() - always returns true (as will never have children); String toString() - Returns a string representation of the primitive; String getShortName() - returns a shortened version of the toString() method |
| debugassist.model | class ObjectVariable extends Variable | Represents an Object (which contains references to other Variables) | None | (The same as PrimitiveVariable) plus: Map<String, Variable> getReferences() - returns all the Variables which this variable refer to; void addReference(String name,Variable ref) - Adds an object to the collection |
| debugassist.pseudobreakpoint | interface PseudoBreakpoint extends ILineBreakpoint (Eclipse Model) | An interface to represent a breakpoint which will be used by Eclipse (for use in the text editor) and by the programming language (to actually insert a breakpoint into the VM) | None | None |
| debugassist.pseudobreakpoint | class PseudoBreakpointCreator | Contains a factory method to determine which kind of PseudoBreakpoint type to create depending on the programming language in use. | None | static PseudoBreakpoint factoryMethod(IResource resource, String typeName, int lineNumber) - Determines which PseudoBreakpoint type to create |
| debugassist.pseudobreakpoint | class PseudoBreakpointJava extends JavaLineBreakpoint (Internal Eclipse JDT breakpoint representation) implements PseudoBreakpoint | A class to represent a Pseudobreakpoint for Java. | None | public PseudoBreakpointJava(final IResource resource, final String typeName, final int lineNumber) - (constructor) - Creates a breakpoint representation, and marker (for use in the text editor); String getMarkerMessage(boolean conditionEnabled, String condition, int hitCount, int suspendPolicy, int lineNumber) - Modifies the name of the Pseudobreakpoint (for identification only); |
| debugassist.pseudobreakpoint | class PseudoBreakpointPython implements PseudoBreakpoint | A class to represent a Pseudobreakpoint for Python. | None | Incomplete |
| debugassist.pseudobreakpoint | class PseudoBreakpointRulerActionDelegate extends AbstractRulerActionDelegate | The delegate class used when the ruler menu action 'Add/Remove Pseudobreakpoint' is used | None | IAction createAction(ITextEditor editor,IVerticalRulerInfo rulerInfo) - Creates a new PseudoBreakpointRulerAction |
| debugassist.pseudobreakpoint | class PseudoBreakpointRulerAction extends Action (inner class of PseudoBreakpointRulerActionDelegate) | The class used when the ruler menu action 'Add/Remove Pseudobreakpoint' is used | None | PseudoBreakpointRulerAction(ITextEditor e, IVerticalRulerInfo r) - constructor - Creates the menu action; void runWithEvent(Event event) - the method which is called when the 'Add/Remove Pseudobreakpoint' menu item is used. Creates a Pseudobreakpoint and adds it to the Eclipse breakpoint manager or will delete the Pseudobreakpoint (if one already exists) |
Design Patterns Used
Observer - I used the Observer pattern in the event handling system.
- Subject = Subject<T>
- ConcreteSubject = BreakpointEventData
- Observer = DataUpdateEventObserver
- ConcreteObserver = BreakpointEventDataStore
There is one slight adjustment to the standard Observer pattern - the ConcreteObserver is told about new data (rather than them querying the ConcreteSubject) - see Tell don't ask. This was done to avoid the ConcreteSubject recording the state information, and instead notifying subjects of new directly information. It also means that we can easily multi-thread the application without worrying about clients missing data.
The current design only has a single observer (a simple data store to record all the breakpoints because visualisations are generated after runtime). You can imagine that it would be easy to implement another visualisation to be generated during runtime using the current Observer pattern.
Composite - I used a Composite pattern to create the notion of a Variable.
- Component = Variable
- Leaf = PrimitiveVariable
- Composite = ObjectVariable
A Variable can either be a primitive or an object. Primitives types in Java are: byte, short, int, long, float, double, boolean, char. Object types are composed of other Variables (other objects or primitives).
Design Maxims Followed
Acyclic dependencies principle - There is no cycles between packages.
Don't expose mutable attributes - All collections have been modified so they are returned as unmodifiable collections. While it does not provide any additional security, it avoids the data collections from being accidentally modified.
Avoid equals - There is currently no equality test between objects. For example, variable objects are only created once and stored in a collection. Nothing else could ever be exactly the same (even if the variable contained the same name, type and value) - it would be a different collection time.
Getters and setters - All variables are hidden and only accessed by the minimum required getters and setters. For example, a variable can never change its name once constructed (so only a getter), but it can change its value (both getters and setters)
Single responsibility principle - Each class have a distinct and separate responsibility.
Hide your decision - Collections are hidden behind the most generic type possible (eg list etc).
Design Maxims Considered
Singleton - In the initial designs there was a lot of Singletons used (example here: .....). However, as the design progressed, it became obvious that this was not entirely necessary. Rather than have a Singleton for each of the classes, only a single singleton was required (in the Activator class). Whenever the plugin is loaded, the Activator class is loaded (this and that happens)... When the plugin is closed, Eclipse also tells the Activator class to shutdown (removing all references to all the other objects) - and thereby clearing the memory.
Design Maxims Violated
Big design up front - There has been three (documented) iterations of development. This project began out of prototyping and evolved into the current product, it was therefore impossible to complete a big design up front as I was unaware of what I would need down the track.
Avoid inheritance for implementation - PseudoBreakpointJava extends Eclipses Java Breakpoint implementation to provide our PseudoBreakpoint for the Java Language. It would have been significantly harder for our PseudoBreakpoint to re-implement our own Java Breakpoint.
Don't burn your base class - The class BreakpointEventData extends Subject to provide the observer functionality (also see Avoid inheritance for implementation). I cannot think of a situation where BreakpointEventData would need to subclass anything other than Subject (famous last words.) The current Java implementation of listeners is exactly the same as my design except I use generics (I'm not saying that Java was designed well!) The BreakpointEventData class only notifies other classes when breakpoint events occur.
The Interesting(??)
- Variable / ObjectVariable / PrimitiveVariable
- Class Encapsulation: Is broken. Variables within the 'Variable' class are marked as protected so subclasses can access them.
Files
Media:debugassist-swe_init.zip - The initial design.
Installation
- Download & Install Eclipse 3.5.2 (with SDK)
- Extract the zip file of my project
- Run Eclipse
- Click File -> Import -> Existing Projects into Workspace
- Select root directory of extracted plugin
- Click Finish
- Create a new Run Configuration: 'Eclipse Application'
