PaulDesignStudy

This layout was copied from Janina's design.

For my research project for Cosc366 I developed a simple GUI that would allow a user to construct MPML3D scripts that control Non Player Characters (NPCs) in 3D Virtual World. I was under pressure to complete the project so I basically threw design out the window and started hacking away. By chance I did implement some parts that were not too smelly but would be unable to tell what pattern they followed.
I will first decide on the requirements for my program and what I would like the program to do. I will give a brief explanation of MPML3D. Then, present and critique my current version to help me to highlight areas I feel require refactoring.

Requirements

• The GUI needs to be intuitive and easy to use.
  
• The GUI must produce valid lines of MPML3D script.
  
• The GUI must incorporate all the options under MPML3D.
  

Constraints

• I cannot change the MPML3D language syntax and semantics or change its code.
  

MPML3D

MPML3D is a XML-based scripting language to describe the behaviour of computer controlled agents. Although it is not bound to a single platform by design, the current version only supports avatars inhabiting Second Life (SecondLife).It can be extended to OpenSimulator as well (OpenSim). With MPML3D, it is easy to construct agents that can give appealing presentation, including speech, gestures and movement. For a more in-depth explanation of the language goto Manual.

Initial Design

Classes

MPML3DGUI.Classes
NPCCreator –
Class that has the main runs the whole application. Basically creates an instance of MainGui and displays it.

Entity –
An Entity is a representation of a Second Life avatar. It has all the information required to log on to Second Life.

MPML3D.Classes.gui
MainGui –
This creates the parent GUI that the other types of GUI get created and displayed within. This class runs the whole show and creates the HeaderGui and BodyGui objects. When the script is ready to be written this class creates an MPML3DWriter and passes it the HeaderGui and BodyGui to be written to a script.

HeaderGui –
This GUI is used by the user to create the header of the script. It holds a list of Entities which are used to write to the script and a list of strings that have are used to write out the External perceptions created. This class also creates an instance of MPML3DHeader.

BodyGui –
The user uses this GUI to create the body of the script. This class creates an instance of an MPML3DBody.

MPML3D.Classes.Writer
MPML3DHeader –
This class has all the data required to write the header of a valid MPML3D script. It has a list of entities and a list of external perceptions.
MPML3DBody –
This class has all the data required to write the body of a valid MPML3D script. It has a list of activities that can also have a list of activities nested with in it.
MPML3DWriter –
This class gets the MPML3DHeader and MPML3DBody classes from the MainGui and writes all the data from the header and body to a file.

MPML3D.Classes.activities
These classes inherit from the AbstractActivity class. Each activity contains the data to create the required lines of valid MPML3D code. Each activity performs a unique action on the avatar.
AbstractActivity
ActionActivity
ParallelActivity
PerceptionActivity
SequentialActivity
SelectedActivity
TaskActivity
For more information of what each activity does please refer to the manual Manual.

Walkthrough

When first running the program the MainGui interface is presented. From here the user can create a new script or edit an existing script. Once an option is selected a HeaderGui window is opened and the user can create entities and external perceptions that they require. Once finished the user clicks the “Create body” button. This calls a method within the MainGui that opens a BodyGui. The user can then enter the activities that control the entities here. When finished the user clicks “Write script” where a file browser opens and the user can navigate to the required folder, as well as naming the script.

==Design Critique==
There are a number of problems with the initial design of my program. The code is quite complex, with long methods and large classes which hints at the fact that I should really refactor and break the visitors up into several classes.

Specific design maxims that are violated by the initial design:

• Avoid downcasting / Beware type switches: In quite a few places in my program, I check if I am looking at a method or a field using the instanceof operator. I then downcast from a Decl to an OperationDecl or FieldDecl before carrying on my analysis. This smells fishy to me. The downcasting suggests that maybe I should use subclassing instead.

• Large class smell / Split large classes: Some of the visitors have a lot of code in them, especially AccessTighteningVisitor and FieldSeparationVisitor which both deal with the relatively complex task of modifying the parsetrees. EncapsulationAnalysisVisitor on the other hand is also large because it tries to record a lot of different data. As such, it has a large number of instance variables and long but simple methods which record data in these instance variables.

• Long method smell / Reduce the size of methods: Some of the methods in the visitors are very long. In EncapsulationAnalysisVisitor, some methods are long but quite simple, consisting of a number of conditional statements to make decisions about what data should be recorded. In AccessTighteningVisitor and FieldSeparationVisitor, some methods are long and relatively complex because they deal with the long-winded process of modifying parsetrees. This requires quite a few different steps, resulting in long methods.

• Duplicate code smell / Don't repeat yourself / Once and only once: There are a number of utility methods for modifying parsetrees and walking the model that appear in more than one of the visitors. For some reason, I was too lazy to pull them out into a separate class so I just copied and pasted them. I know that's really bad...

• Single responsibility principle / One responsibility rule / One key abstraction: I feel like some of the visitors are so large because they try to do too much. AccessTighteningVisitor and FieldSeparationVisitor both figure out which parts of the parsetree should be modified and then go on to do the modification. It seems like they have more than one responsibility and as a result, they have become quite large.

• Distribute system intelligence: This is closely related to the last point about some of the visitors having too much responsibility. I don't think that I have divided system intelligence properly but instead concentrated it in the large visitor classes.

• Sequential coupling: This is an anti-pattern that says that a class should not expect its clients to call methods in a particular order. This is currently the case in my design, where Main first creates a visitor, then tells it to start visiting the model, then potentially tells it to resolve accesses and finally tells it to do something with the results. This seems to lead to pretty tight coupling between Main and the visitors and should be removed.

• Keep related data and behavior in one place: In my design, the behavior for reshaping parsetrees has been separated from the parsetrees and the behavior for visiting the model has been separated from the model. This violates Riel's heuristic or keeping related data and behavior in one place. However, one of the constraints of the project is that I cannot change the JST class so that I cannot put that behavior with the data it acts on so there is little I can do about this heuristics breach.

Aims for my first design

• Break up the large classes and methods so that each class has one clear responsibility and system intelligence is distributed better.
• Get rid of all the duplicated code and put it somewhere sensible.
• Try to use subclassing to avoid downcasts in the code. This means somehow handling fields and methods separately.
• Remove the horrible sequential coupling from the visitors' interfaces to reduce coupling between Main and the visitors.