Aidan's Design Study
(→Violated Principles) |
|||
Line 60: | Line 60: | ||
− | === | + | === Rough Notes: Delete this later === |
==== [[Long method smell]] / [[Switch statement smell]] ==== | ==== [[Long method smell]] / [[Switch statement smell]] ==== | ||
* Process.setState(ActionEnum) and Process.setState(Point,int) are massive methods | * Process.setState(ActionEnum) and Process.setState(Point,int) are massive methods |
Revision as of 09:37, 10 September 2009
Part of my honours project is to implement two interfaces for an Intelligent Tutoring System (ITS) for Thermodynamics (tentatively named Thermo-Tutor). The original interface basically aims to make input as efficient as possible, however in an ITS this shouldn't be the primary goal; ITSs are meant to teach something. The experimental interface takes some ideas from the field of cognitive science and sacrifices a certain amount of efficiency for improved learning (hopefully). From an OO design perspective the two interfaces are not drastically different, so I will only be looking at the standard interface. In my haste to get the interface working I invested very little effort in design. I'm sure that upon closer inspection I will find all sorts of stupid decisions.
Thermo-Tutor Interface
The interface of Thermo-Tutor is primarily a Java applet. The applet has three main components as shown in the following screen shot:
Area 1: Diagram
Here the student is required to draw a diagram of the thermodynamic cycle described in the problem text (not shown). The diagram shows states (as numbered circles) and transitions between those states (labeled according to the states they join). States [1] represent points in the cycle where some properties are known (pressure, volume and/or temperature). Transitions represent the change in the state of the cycle between these states. Each transition is characterised by one property remaining constant throughout that transition. For example the transition 2 - 3 in the diagram is an Isochoric (constant volume) transition.
Area 2: Data Summary
The Data Summary has two purposes. Its main purpose is to store and display all the information that is so far known about the cycle for use in the following step. Its secondary purpose is to allow the student to specify some constants which will also be useful in the following step. The values in the center of the data summary (Cv, Cp etc) are specified explicitly by the student. The other values are entered by the student as they are drawing the diagram. The empty cells in this table are the values which will be calculated in the following step.
Area 3: Formula Workspace
The Formula Workspace is where the student will spend the majority of their time when solving a problem. The previous two steps essentially required the student to deduce some information from the problem statement, no calculations were required. The Formula Workspace allows the student to incrementally calculate the values of properties displayed in the Data Summary. Basically the student is required to complete the Data Summary by calculating each value using some formula. The formulas are predefined in the applet. During this step the student repeatedly iterates through several sub-steps:
- Select the unknown to calculate
- Select the formula to use
- Rearrange the formula to have the correct subject
- Fill in the formula with values from the Data Summary
Each iteration through the steps above will add one more value to the Data Summary, which should hopefully allow subsequent calculations.
Requirements
- Intuitive Interface: In order to facilitate transferable learning it is important that the interface corresponds closely to how students would solve similar problems on paper.
- Maintain Working Solution: The applet must maintain a model of the student's working solution which can be easily manipulated and exported to XML to be sent to the server for checking.
- Maintainability and Extensibility: If Thermo-Tutor is to be extended to support more complex problems in the future, it is likely that the applet will need to be altered also.
Constraints
- None so far: What limits the design in any way? Any principles that I have chosen to break because it's unavoidable?
Initial Design
For my design study I will be looking at the design of the model in the applet, as opposed to the view. The following is a UML Class Diagram of the design which occurred very much by accident.
Description of Classes
- Process: A thermodynamic cycle
- Element: Something which can be placed on the diagram
- Movable: Something which can be moved on the diagram
- StateElement: A state in the cycle
- Transition: A transition in the cycle
- AdiabaticTransition: Constant energy transition
- IsobaricTransition: Constant pressure transition
- IsochoricTransition: Constant volume transition
- IsothermalTransition: Constant temperature transition
- Handle: Points of components on the diagram which can be joined together
- TransitionHandle: A point on a Transition which can be joined to a StateHandle
- StateHandle: A point on a StateElement which can be joined to a TransitionHandle
- Unknown: A property which is not given in the problem text
- TransitionUnknown: A property of a transition which is not given in the problem text
- StateUnknown: A property of a state which is not given in the problem text
- Formula: A formula which can be used to calculate an unknown
- SpecialValue: A value which can be deduced from the problem text, but is not inherently part of the cycle. These are selected on the DataSummary (see Area 2 above).
Rough Notes: Delete this later
Long method smell / Switch statement smell
- Process.setState(ActionEnum) and Process.setState(Point,int) are massive methods
- Concrete Unknowns have switch smell around setting/clearing values. Adding more unknowns would be made more difficult by this. This problem is centered around the fact that Unknowns have an enum StateUnknown_Type or TransitionUnknown_Type. This is simply disgusting.
- Concrete Unknowns both have getUnknownUnit() method. These are actually only being used for XML export. This is basically to encapsulate the switch smell.
Separation of concerns
- Should Element (and other things) be painting and exporting themselves?
- All concrete Transitions export themselves to XML, but they do call methods defined in Transition to do some of the work
Liskov substitution principle
- Element has begin/end point, yet this doesn't make sense for a StateElement
- Delta is also defined in Element, yet this is only really a property of a Transition
Interface should be dependent on model
- Element also defines methods for selection. This is really a function of the view.
- The whole Movable interface is actually a function of the view.
- Handle.setCursorType() is another property that belongs to the view
- Process holds a reference to ProcessApplet: the model is dependent on the view (other public fields also indicate this)
- Process has methods for opening State and Transition dialogs, again this is view stuff.
- Formula.imageComponent() takes a JApplet. It would be better if this were not necessary; ties the model to the view.
- SpecialValue.imageComponent() takes a JApplet similar to Formula
One key abstraction
- Both Element and Movable define a method prepareForDeletion(). This is needed for Handle, but still suggests something iffy.
- There is certainly some issue surrounding the distinction between values known and values calculated.
- Formula seems to take two roles; Formula and Expression
Avoid interface bloat
- Handle.moveHandleResizingOwner() is only used privately by TransitionHandle
- Handle: isConnected() wasConnected() seems a little wierd. Encapsulation leak?
- Think about how connecting/disconnecting should be done. Currently much of it is overseen by Process.
- Process.checkTransitionVolumeIntegrity and Process.checkTransitionPressureIntegrity: This could be done in the Transition, possible automatically when it is necessary, but atleast done inside Transition.
- Process.assignNextLabel() and assignTransitionLabel() could both be done else where. Also it seems default state and transition labels are assigned in completely different manners.
- Process.isStateTransitionUnique() is another check that may be better elsewhere
- SpecialValue doesn't contain much of the behaviour. Most of the behaviour has leaked out into the view.
Hide data within its class
- Both StateElement and Transition have get/set ID. If this were to be done at all (which it shouldn't) it should be done in Element.
- Process.getNextID() is awful. IDs should be passed in at construction time and they should never change.
- Process has some public fields
Avoid no-op overrides / Refused bequest smell
- Implementation of move(Point) in all concrete Transition types is empty
Just Silly
- Handle.pointIsInHandle() returns itself, instead of a boolean
- Only StateElement has a setHandles() method, not Transition
- Both StateElement and Transition have getUnknowns() and getCalculatedValues() methods, perhaps this should be in Element?
- All concrete Transitions have a pointIsInElement() that returns itself. Should return boolean, or not exist at all?
Steps to Take
This is a mess, so let's break it down a little.
- Sort out the Unknown class and it's subclasses.
- We should store all element properties in this fashion. Values that are both specified and calculated should be stored in the same way.
- Is it important to distinguish between values that are specified in the first step and those that are calculated in the third step?
- Process looks like a god class.
- It seems to be managing states and transitions too much
- Issues from the view have leaked into the model