PaulDesignStudy

From CSSEMediaWiki
(Difference between revisions)
Jump to: navigation, search
(edited page to have my stuff instead of Janina)
(Final Layout)
 
(32 intermediate revisions by 3 users not shown)
Line 2: Line 2:
  
 
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.
 
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 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.<br>
 
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.
 
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==
 
==Requirements==
The GUI needs to be intuitive and easy to use.
+
* The GUI needs to be intuitive and easy to use.<br>
The GUI must produce valid lines of MPML3D script.
+
* The GUI must produce valid lines of MPML3D script.<br>
The GUI must incorporate all the options under MPML3D.
+
* The GUI must incorporate all the options under MPML3D.<br>
  
 
==Constraints==
 
==Constraints==
* I cannot change the MPML3D language syntax and semantics or change its code.
+
* I cannot change the MPML3D language syntax and semantics or change its code.<br>
  
 
==MPML3D==
 
==MPML3D==
Line 19: Line 19:
 
==Initial Design==
 
==Initial Design==
  
UML diagram of initial design to come.
+
[[image:uml1.png|center|frame|Original UML]]
 
+
 
===Classes===
 
===Classes===
MPML3DGUI.Classes
+
MPML3DGUI.Classes<br>
NPCCreator.
+
NPCCreator –<br>
Class that has the main runs the whole application. Basically creates an instance of MainGui and displays it.
+
Class that has the main runs the whole application. Basically creates an instance of MainGui and displays it.<br>
  
Entity –
+
Entity –<br>
An Entity is a representation of a Second Life avatar. It has all the information required to log on to Second Life.
+
An Entity is a representation of a Second Life avatar. It has all the information required to log on to Second Life.<br>
  
MPML3D.Classes.gui
+
MPML3D.Classes.gui<br>
MainGui –
+
MainGui –<br>
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.
+
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.<br>
  
HeaderGui –
+
HeaderGui –<br>
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.
+
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.<br>
  
BodyGui –
+
BodyGui –<br>
The user uses this GUI to create the body of the script. This class creates an instance of an MPML3DBody.
+
The user uses this GUI to create the body of the script. This class creates an instance of an MPML3DBody.<br>
  
MPML3D.Classes.Writer
+
MPML3D.Classes.Writer<br>
MPML3DHeader –
+
MPML3DHeader –<br>
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.
+
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.<br>
MPML3DBody –
+
MPML3DBody –<br>
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.
+
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.<br>
MPML3DWriter –
+
MPML3DWriter –<br>
This class gets the MPML3DHeader and MPML3DBody classes from the MainGui and writes all the data from the header and body to a file.
+
This class gets the MPML3DHeader and MPML3DBody classes from the MainGui and writes all the data from the header and body to a file.<br>
  
MPML3D.Classes.activities
+
MPML3D.Classes.activities<br>
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.
+
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.<br>
AbstractActivity
+
AbstractActivity<br>
ActionActivity
+
ActionActivity<br>
ParallelActivity
+
ParallelActivity<br>
PerceptionActivity
+
PerceptionActivity<br>
SequentialActivity
+
SequentialActivity<br>
SelectedActivity
+
SelectedActivity<br>
TaskActivity
+
TaskActivity<br>
For more information of what each activity does please refer to the manual [http://research.nii.ac.jp/~prendinger/MPML3D/MPML3D.html#MPML3D_Reference_Manual Manual].
+
For more information of what each activity does please refer to the manual [http://research.nii.ac.jp/~prendinger/MPML3D/MPML3D.html#MPML3D_Reference_Manual Manual].<br>
  
  
Line 63: Line 62:
 
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.
 
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==
+
==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.  
+
I feel there are a lot of things that can be improved with in this design.
  
 
Specific design maxims that are violated by the initial design:
 
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.
+
*[[Avoid_no-op_overrides | No-op overrides]]/[[Design_by_contract|Design by contract]]/[[Liskov_substitution_principle|Liskov Principle]]: Some of the subclasses of AbstractActivity donot require some of the super classes methods and have been overwritten with a stub that says this method not required.
  
*[[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.
+
*[[Coupling_and_cohesion|Coupling]]: The Gui and backend are strongly coupled making it hard to create a new Gui for the system.
  
*[[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.
+
*[[Data_class_smell|Data class smell]] / [[Eliminate_irrelevant_classes|Eliminate irrelevant classes]]:A few of the subclasses of AbstractActivity do not require any additional functionality.
  
*[[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...
+
*[[Avoid downcasting]] / [[Beware type switches]]: I sometimes need to check what activity I have and cast them to a type. This is ugly.
  
*[[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.
+
==Aims for my first design==
  
*[[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.
+
* Create a MVC pattern
 +
* Use a composite design pattern for AbstractActivity and subclasses
 +
* or try and remove the no-op methods
  
*[[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.
+
==AbstractActivity and subclasses.==
 +
[[image:firstactivityuml.jpg|center|frame]]<br><br>
 +
The parallel and selected activity classes have no additional functionality to the super class. The ActionActivity cannot contain any other activities so the add remove activity method from the super are no ops. My first guess is to use a Composite design pattern but I feel as though I need to move some of the classes around. On closer inspection the PerceptionActivity should not be here as I misread the information on what a perception is so I will move this class to a new hierarchy with two other classes.<br>
 +
[[image:elementumql.jpg]]<br><br>
 +
By following a similar pattern for the AbstarctActivity hierarchy I run into the problem of what to do with TaskActivity as it can add Activites to its list but cannot be part of any other activities list.<br>
 +
<br>[[image:nextactivityuml.jpg|center|frame]]<br><br>
  
*[[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.
+
At Paul's request... I came up with an adjustment to the design. --[[User:Matthew Harward|Matthew Harward]] 01:48, 6 August 2009 (UTC)
  
==Aims for my first design==
+
[[Image:Paulsuml.png|thumb|left|A specialised inhertance hierachy using a [[Decorator]].]][[Image:Paulsuml2.png|thumb|center|Using an interface to avoid no-ops.. Does this work?]]
 +
 
 +
[[Image:IWrapable_version.png|thumb|left|Yet another option --[[User:Michal Connole|Michal Connole]] 02:41, 6 August 2009 (UTC)]]<br>
 +
 
 +
<br><br><br><br><br><br><br><br><br><br><br>Thanks to Michal, Mathew and class for there input here is the final design that I choose I thought I was close to a solution with my AbstractElement solution but by adding the interface wrapable it helped to fix the problem with task activity with it being able to hold other activities but could not actually be wrapped.<br><br>
 +
[[image:AbstractElement2.jpg]]
 +
 
 +
==Model-View-Controller==
 +
The model consists of the MPML3DHeader and MPML3DBody. The view consists of a all encompassing starting GUI that can has a HeaderGui and a BodyGui. For now I will not concentrate on where the heck the MPML3DWriter should be as I am as of yet undecided.
 +
 
 +
Will a clear cut between model and viewer i now need to create a controller and decide how to design it. Using it more as a mediator pattern I have decided to break it up in two separate controllers as they control two separate things the header and the body of the script.
 +
 
 +
[[image:MVC1.png|center|frame]]<br><br>
 +
 
 +
==Followed design principles==
 +
*[[Avoid downcasting]] : This was accomplished using the way Wal does it
 +
 
 +
*[[Single responsibility principle]] / [[One responsibility rule]] : This is the main reasoning for having a MPML3D header, body and writer. Each one of these classes takes care of the 3 distinct areas of a MPML3D script.
 +
 
 +
*[[Keep related data and behavior in one place]]: Also for the above reasonings.
 +
 
 +
*[[Avoid no-op overrides]] : By using a variation of the composite design pattern and using a wrapable interface the problem of no ops has been addressed.
 +
 
 +
*[[MVC]] : Adding the MVC pattern allows the backend and gui to be totally uncoupled making it easier to create a new front end gui.
 +
 
 +
*[[Open closed principle]] : By using inheritance for the activitys and elements it is easy to add new members to these hierarchys.
 +
 
 +
==Design patterns used==
 +
 
 +
* [[Composite]] : This pattern was used twice with a slight alteration in than all leaf values are placed outside the composite part. It was used for activities and elements
 +
 
 +
* [[MVC]] : This pattern was used to totally decouple front and back ends
 +
 
 +
==Design conflicts==
 +
* [[Eliminate irrelevant classes]] - I have created a couple of subclasses selected and parallel that do nothing more than what the super class does. Although this goes against this hieristic I feel for better understanding and separating the activities apart I have decided to keep this classes in the design.
 +
 
 +
==Summary==
 +
I feel the new design has a more pleasing or less smelly feel to it. With the use of the MVC the whole system just looks a lot less coupled. As well as the modified composite design patterns adding extensibility as well as freeing from no ops.
 +
 
 +
 
 +
==Final Layout==
 +
 
 +
[[image:finals.png|center|frame]]
  
* Break up the large classes and methods so that each class has one clear responsibility and system intelligence is distributed better.
+
[[Media:ScriptWriterMPML3D.zip]]
* 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.
+

Latest revision as of 01:13, 5 October 2009

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.

Contents

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

Original UML

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

I feel there are a lot of things that can be improved with in this design.

Specific design maxims that are violated by the initial design:

  • Coupling: The Gui and backend are strongly coupled making it hard to create a new Gui for the system.

Aims for my first design

  • Create a MVC pattern
  • Use a composite design pattern for AbstractActivity and subclasses
  • or try and remove the no-op methods

AbstractActivity and subclasses.

Firstactivityuml.jpg


The parallel and selected activity classes have no additional functionality to the super class. The ActionActivity cannot contain any other activities so the add remove activity method from the super are no ops. My first guess is to use a Composite design pattern but I feel as though I need to move some of the classes around. On closer inspection the PerceptionActivity should not be here as I misread the information on what a perception is so I will move this class to a new hierarchy with two other classes.
Elementumql.jpg

By following a similar pattern for the AbstarctActivity hierarchy I run into the problem of what to do with TaskActivity as it can add Activites to its list but cannot be part of any other activities list.


Nextactivityuml.jpg


At Paul's request... I came up with an adjustment to the design. --Matthew Harward 01:48, 6 August 2009 (UTC)

A specialised inhertance hierachy using a Decorator.
Using an interface to avoid no-ops.. Does this work?
Yet another option --Michal Connole 02:41, 6 August 2009 (UTC)












Thanks to Michal, Mathew and class for there input here is the final design that I choose I thought I was close to a solution with my AbstractElement solution but by adding the interface wrapable it helped to fix the problem with task activity with it being able to hold other activities but could not actually be wrapped.

AbstractElement2.jpg

Model-View-Controller

The model consists of the MPML3DHeader and MPML3DBody. The view consists of a all encompassing starting GUI that can has a HeaderGui and a BodyGui. For now I will not concentrate on where the heck the MPML3DWriter should be as I am as of yet undecided.

Will a clear cut between model and viewer i now need to create a controller and decide how to design it. Using it more as a mediator pattern I have decided to break it up in two separate controllers as they control two separate things the header and the body of the script.

MVC1.png


Followed design principles

  • Avoid no-op overrides : By using a variation of the composite design pattern and using a wrapable interface the problem of no ops has been addressed.
  • MVC : Adding the MVC pattern allows the backend and gui to be totally uncoupled making it easier to create a new front end gui.
  • Open closed principle : By using inheritance for the activitys and elements it is easy to add new members to these hierarchys.

Design patterns used

  • Composite : This pattern was used twice with a slight alteration in than all leaf values are placed outside the composite part. It was used for activities and elements
  • MVC : This pattern was used to totally decouple front and back ends

Design conflicts

  • Eliminate irrelevant classes - I have created a couple of subclasses selected and parallel that do nothing more than what the super class does. Although this goes against this hieristic I feel for better understanding and separating the activities apart I have decided to keep this classes in the design.

Summary

I feel the new design has a more pleasing or less smelly feel to it. With the use of the MVC the whole system just looks a lot less coupled. As well as the modified composite design patterns adding extensibility as well as freeing from no ops.


Final Layout

Finals.png

Media:ScriptWriterMPML3D.zip

Personal tools