Guitar Amplifier Design
(→Implementing my second design) |
m (→Related Links) |
||
(36 intermediate revisions by one user not shown) | |||
Line 51: | Line 51: | ||
== My second attempt == | == My second attempt == | ||
− | [[Image:the second design.jpg]] | + | [[Image:the second design.jpg|800px]] |
This is my second version of the design. I've implemented the Strategy design pattern in two places to handle the switching between Delay effects and the switching between Normal effects. Strategy seemed like the best way to model this because Strategy works well when you have many related classes that differ in behaviour. The processSignal() method that can be seen in each concrete effect class and in the abstract Effect class is there to represent the algorithm that would be implemented differently by each concrete class. The processing of an actual sound signal is a bit outside the scope of this design project but I've put that method there to properly convey the use of the Strategy pattern. | This is my second version of the design. I've implemented the Strategy design pattern in two places to handle the switching between Delay effects and the switching between Normal effects. Strategy seemed like the best way to model this because Strategy works well when you have many related classes that differ in behaviour. The processSignal() method that can be seen in each concrete effect class and in the abstract Effect class is there to represent the algorithm that would be implemented differently by each concrete class. The processing of an actual sound signal is a bit outside the scope of this design project but I've put that method there to properly convey the use of the Strategy pattern. | ||
Line 77: | Line 77: | ||
If I do it this way, then my classes will potentially become victims of the [[Large_class_smell]] because the ControlPanel and Channel classes will have the methods that the Settings class has. Similarly, the ControlPanel object will end up with a fat interface (see: [[Fat interfaces]]). | If I do it this way, then my classes will potentially become victims of the [[Large_class_smell]] because the ControlPanel and Channel classes will have the methods that the Settings class has. Similarly, the ControlPanel object will end up with a fat interface (see: [[Fat interfaces]]). | ||
− | '''Result:''' I was in a situation where a few trade-offs and opposing forces appeared. The first being whether I should follow the [[Law of Demeter]] and [[Tell, Don't Ask]] | + | '''Result:''' I was in a situation where a few trade-offs and opposing forces appeared. The first being whether I should follow the [[Law of Demeter]] and [[Tell, Don't Ask]] or Ken Auer's recommendation of always using [[Getters and setters]]. The second trade-off that appeared was that using the Law of Demeter meant that my ControlPanel class would end up with a fat interface ([[Fat interfaces]]). I've decided to go with the [[Law of Demeter]] for two reasons. The first being that using [[Getters and setters]] to access things that are at deeper levels in my design, creates a lot of coupling between classes. This is explained below: |
− | + | ControlPanel.getCurrentChannel().getSettings().setAmpModel("RectoDual"); | |
+ | |||
+ | This code lets the ControlPanel object directly modify a variable stored within the Settings object, so this makes the ControlPanel object become dependant on the internal structure of the Settings class. It is much better to use [[Law of Demeter]] because this means if the Settings class was changed, then only the Channel class would have to have code modified. | ||
+ | |||
+ | |||
+ | Secondly, [[Fat interfaces]] states that: "A fat interface is usually the one which is aimed at serving more than one set of clients." but this statement doesn't apply to the fat interface created by in the ControlPanel class by using the [[Law of Demeter]]. The reason being that the interface the ControlPanel class will have, although large, is the same interface that the real version of the Flextone provides and doesn't attempt to serve more than one set of clients. So in a way, having a ControlPanel class with a fat interface is sticking to the design maxim of [[Model the real world]]. | ||
+ | |||
+ | The most interesting thing that arose from this situation is that even though I chose to use the [[Law of Demeter]], my code still has [[Getters and setters]]! This is interesting because the [[Law of Demeter]] and [[Getters and setters]] are supposed to be conflicting ideas, but I have happened to use both ideas in my design. I wanted to use [[Law of Demeter]] and [[Tell, Don't Ask]] because of the reasons stated above but what about [[Model the real world]]? Most of the actions performed when using the real-life version of the amplifier are simply setting different variables to have different values, e.g. setting the volume, setting the tempo of the delay and so on. Because of this, it makes sense to use [[Getters and setters]] in my design to allow the user to perform these actions. My design is using [[Law of Demeter]] but doesn't completely align with the idea of [[Tell, Don't Ask]] because my ControlPanel class has getter (asking) methods that simply tell the Channel class to do the asking for it. So in a sense, I managed to balance out conflicting forces. | ||
+ | |||
+ | == Third design == | ||
+ | |||
+ | My idea of implementing the second design in order to discover more things that could be improved worked quite well, and as mentioned above I struck upon a few conflicting forces during the implementation. Below is my third class diagram which shows the results of the implementation. Not too much has been changed in terms of new classes being made but [[Law of Demeter]] did help to clarify the behaviour of the design. | ||
+ | |||
+ | [[Image:thirdAttempt.jpg|800px]] | ||
+ | |||
+ | ==== A critique of my third design ==== | ||
+ | |||
+ | Overall, I believe that my third design is quite good because it encompasses all the core functionality that is provided to a user in the real-world version of the amplifier. You can select channels, select amp and cabinet models, select effects and save a group of settings to each channel. But there are a few things aren't quite right yet... | ||
+ | |||
+ | For example, my use of the [[Strategy]] design pattern works quite well to handle the switching between different types of effects. But it doesn't handle the situation where you aren't using any effects. This is where the [[Introduce Null Object]] refactoring technique is useful. | ||
+ | |||
+ | Another problem is that when selecting a new amp model, the design doesn't constrain you to only using the amp models it actually has. Currently it doesn't store a list of AmpModel objects that your amp selection is checked against. To further explain this problem, if you used the command "Select Amp Banana", the current design would simply create a new AmpModel object with the name "Banana". What the design should really do is check your selection against a pre-defined list of AmpModels. If this constraint was implemented then I could also implement some method called createNewAmpModel() or something similar that allowed you to add new amp models to the list. | ||
+ | |||
+ | Continuing on from the idea of being able to add new amp models, it makes sense that you should be able to have more than 4 different channels to choose from. Currently the ControlPanel constructor creates a new Hashtable and populates it with 4 channel objects. But I think it would be nice if you could dynamically add more channels as you are using the program instead of this option being hard-coded into the constructor. Maybe this calls for a [[Factory Method]] but there might be a different way of doing this as well. | ||
+ | |||
+ | Also, currently the main point of control is the ControlPanel class. But I think the name of this class should be changed if I implement functionality mentioned above because really it isn't the control panel that knows what amp models the amplifier has but instead is the amplifier itself. So I think Amp or Flextone might be a more suitable name for the class. | ||
+ | |||
+ | == My final design == | ||
+ | |||
+ | Below is the class diagram of my final version of the design | ||
+ | |||
+ | [[Image:finalAttempt.jpg|900px]] | ||
+ | |||
+ | |||
+ | In terms of classes, I have only introduced two new classes. These are the NoDelay and NoEffect classes. By adding these two classes I was able to handle the situation where no delay or effects are in use. This was mentioned earlier in the critique of my third design. I took the idea of [[Introduce Null Object]] and modified it slightly to suit my design. The [[Introduce Null Object]] technique suggests creating an isNull() method in the superclass of the null object and over-riding this method in the null object itself. This means that checks for null in the code can be replaced with calls to isNull() on the objects being used. For my design this isNull() method was not necessary, I just needed a class to represent the concept of no effects or delay being used. The NoDelay and NoEffect methods still implement the processSignal() method but return different output stating that the signal is not being processed. This is the same idea as the signal bypassing the sound processor that would usually modify the signal to create the effect. | ||
+ | |||
+ | During the critique of my third attempt I talked about the idea of using the [[Factory Method]] design pattern to implement new functionality for creating new Channels, AmpModels and so on. Although the [[Factory Method]] is a useful design pattern I decided that it was not needed because it is designed to handle a problem I wasn't actually encountering. The [[Factory Method]] is very useful when a particular class (in my design it's the Amplifier class, formerly known as the ControlPanel) cannot anticipate the class of an object (a new Channel) it must create. In my design the Amplifier class does not need to anticipate what type of object it needs to create because that information is supplied by the user. Instead my final design simply creates a new Channels, AmpModels, CabinetModels and Normal/Delay Effects when the user instructs the program to do so, e.g with the command "Add New Channel". This decision is good because it's important to [[Do the simplest thing that could possibly work]]. | ||
+ | |||
+ | The main() method that I was using to handle user input was getting a bit large so I decided to use the [[Extract Method]] to clean up the code, this made a few new static methods but gave a good result. Even though the main method is still sizeable, I managed to remove about 200 lines of code. The code that is still there is mainly if-statements so it's repetitive but not too messy. | ||
+ | |||
+ | The only feature that doesn't fulfill it's potential is the showSettings() method. This method displays the settings held in the savedSettings object, so if a change is made to some settings, showSettings() won't display this until saveSettings() has been called. saveSettings() is called whenever a user types the command "Save Settings". Although this isn't optimal behaviour, generally a user would modify a large range of settings before saving and would be aware of what settings have been changed. | ||
+ | |||
+ | Overall, I think my final design has managed to capture the fundamental behaviour of the interface of the real Flextone III, and has also managed to extend this behaviour in a few ways with the ability to add new channels, amp models, cabinets and effects. Although the Amplifier class itself has a reasonably large interface, this is understandable due to the nature of the Flextones interface and the use of the [[Law of Demeter]]. | ||
+ | |||
+ | == Source code == | ||
+ | |||
+ | The .zip file below includes all source files for the design study, a read-me explaining the various commands, and the executable .jar file. | ||
+ | |||
+ | [[Media: guitarAmplifier.zip|guitarAmplifierDesignStudy.zip]] | ||
+ | |||
+ | == Related Links == | ||
+ | |||
+ | [[Introduce Null Object]] | ||
+ | |||
+ | [[Law of Demeter]] | ||
+ | |||
+ | [[Tell, Don't Ask]] | ||
+ | |||
+ | [[Factory Method]] | ||
+ | |||
+ | [[Strategy]] | ||
+ | |||
+ | [[Model the real world]] | ||
+ | |||
+ | [[Getters and setters]] | ||
+ | [[Extract Method]] | ||
− | + | [[Large_class_smell]] |
Latest revision as of 03:55, 1 October 2008
Contents |
Guitar Amplifier Design
For my design study I have decided to create a software version of the Line 6 Flextone III guitar amplifier. Although designing the modules that create the actual guitar tone is out of the scope of this project (any electrical/audio engineers around??) my design will model the core behaviour of this marvellous piece of equipment.
The Flextone III is a digital modelling amp which means that it uses digital methods to imitate a wide range of different amplifiers that are available on the market today. It's a jack of all trades!
Behavioural Overview
The amp allows you to choose from 16 different amp models with each model having a primary (yellow) mode and a secondary (red) mode. It also allows you to choose from 16 different speaker cabinet models but the speaker cabinet models do not have a secondary mode (their selection light is just green). The speaker cabinet selection allows you to choose what sort of speakers the currently selected amp model sounds like it's being played through.
The Flextone also has built in effects. There are two groups of effects and each group has 6 different options to choose from. The first group are all Delay (Echo) related effects, and the second group are all general effects such as: Chorus, Flanger, Tremelo, Phaser and so on. The delay group of effects has a variable knob called "Delay" that adjusts how noticeable the effect is when mixed with the original signal. The general group of effects also has a variable knob that does the same thing for those effects but it's called "Mod".
The last feature related to effects is the small "Tap Tempo" button located just above the group of delay effects. This button can be used to set the tempo of the delay effects. The user simply taps the button twice at a certain speed and the tempo is set. This tempo will apply to all 6 delay effects so it's not possible to have different tempos saved on the same channel. There are also 7 different variables that can be modified that affect the sound of your current settings (whatever they might be). These are: Drive, Bass, Mid, Treble, Presence, Volume and Reverb.
Lastly there are two switches that determine whether the sound-gate (kills unwanted background noise) is on or off and whether the compressor (makes all notes have roughly equal volume, great for solos) is on or off.
The Flextone has four different "channels" A, B, C and D that you can save a group of settings on. A group of settings consists of an amp model choice, a speaker cabinet choice, the 7 values of the D, B, M, T, P, V and R variables, the delay effect and general effect you might be using (if any) and the values of their "Delay" and "Mod" knobs respectively. Also included are is current set tempo for the delay effects and finally the status of the sound-gate and compressor (on or off). Once you have saved some settings to a Channel A for example, you can then click the Channel B button and the Flextone will load up the settings related to Channel B. Channel B's settings can be entirely different to Channel A's. You may decide you want to change something about the settings on Channel B so after you make that change you can just save the Channel B settings again.
There is one last feature of the amplifier to be mentioned and that is the Master Volume control. While modifying the Volume variables on each different Channel will set the volumes of each Channel in relation to each other, the Master Volume adjusts the volume of the entire amplifier. This means the volumes of each channel are proportionally adjusted up or down by the master but always have the same relative difference in volume.
A quick example: A has it's volume set to 10. B, C and D have their volumes set to 5. The master volume level is currently 2. This means A's volume level is 2 (based on the measurement of the master volume) and the other three channels have a master volume value of 1. If the master if turned up to 3, then A has a volume of 3 and the other channels have a volume of 1.5.
Above: An overview of the control panel and all the controls described above.
Below: The amp/cabinet selection tool. Yellow and Red names are the primary and secondary modes of each amp and the Green names are the names of the speaker cabinets.
The Beginning of the Design
This is my first attempt at a class diagram for this design. Because the Flextone's behaviour can potentially be very confusing in terms of creating a suitable design I've decided to just pick one direction of design for now and go with it. This will hopefully lead to me to a point where I find some concrete flaws with the design. From there I will be able to start using knowledge gained from lectures and this wiki to improve the design.
A critique of my first design
After looking over my first design I have found a few (expected) flaws related to correctly modelling the behaviour of the Flextone. I created a ControlPanel class which handles overall control of the amplifier e.g. changing the Master volume and which Channel is currently selected, and I also created a Channel class. The Channel class has an attribute called "settings" (with no type defined, it could be a string) and this is part of the first flaw.
I believe there should be a "Settings" class and that the "settings" variable in Channel should be an instance of the Settings class. I think this makes a bit more sense because this way the AmpModel, CabinetModel, DelayEffect and NormalEffect classes will all be related to the Settings class instead of the Channel class. It doesn't make much sense that an AmpModel is a part of a particular channel because all amp models are available to be used regardless of which channel you have selected. If efficiency was an issue then maybe the FlyWeight pattern could come into use and I could just have 16 different AmpModel objects and if more than one channel was using the same AmpModel instance then both channels would reference the same object.
The second problem I noticed is that if the Channel class had one DelayEffect field and one NormalEffect field, then only 1 out of the 6 delay effects could be held and only 1 out of the 6 normal effects could be held. Having one of each type of effect being held in Channel is fine, but the inheritance model in my class diagram isn't correct because the DelayEffect and NormalEffect classes should be abstract and each one should have 6 concrete sub-classes. This is starting to look like a place where the Strategy design pattern could be used.
My second attempt
This is my second version of the design. I've implemented the Strategy design pattern in two places to handle the switching between Delay effects and the switching between Normal effects. Strategy seemed like the best way to model this because Strategy works well when you have many related classes that differ in behaviour. The processSignal() method that can be seen in each concrete effect class and in the abstract Effect class is there to represent the algorithm that would be implemented differently by each concrete class. The processing of an actual sound signal is a bit outside the scope of this design project but I've put that method there to properly convey the use of the Strategy pattern.
Also I added the "Settings" class I mentioned in the first version of this design. The Settings class holds information about which AmpModel, CabinetModel, DelayEffect and NormalEffect are currently selected and also supplies methods to change these fields. These methods represent the idea of a person changing settings using the main selection knob on the Flextone or changing effects using the small selection button below each group of effects.
Implementing my second design
To further improve the quality of my design, I decided the best idea was to start coding my second design up. The reason being that even though the design is fairly simple, implementation may help me discover certain issues about the design and find more things that can be improved. From this implementation I believe I will be able create my third class diagram which will be more elegant.
Law of Demeter: My idea for implementing this design is to have my program read commands from the command line and act upon those commands. This is where I have to make a judgement call about my design. If I wanted to select the AmpModel for a particular Settings object, I might type "Select Amp RectoDual" to switch to the RectoDual model. At this point I can either break the Law of Demeter and the principle of Tell, Don't Ask by having some code like:
ControlPanel.getCurrentChannel().getSettings().setAmpModel("RectoDual");
...which is quite long and messy, or I can create a setAmpModel() method in the ControlPanel class which has some code inside it like:
setAmpModel(String ampName) { current.setAmpModel(ampName); }
In the code above, current is the name of a Channel object that the ControlPanel holds.
If I do it this way, then my classes will potentially become victims of the Large_class_smell because the ControlPanel and Channel classes will have the methods that the Settings class has. Similarly, the ControlPanel object will end up with a fat interface (see: Fat interfaces).
Result: I was in a situation where a few trade-offs and opposing forces appeared. The first being whether I should follow the Law of Demeter and Tell, Don't Ask or Ken Auer's recommendation of always using Getters and setters. The second trade-off that appeared was that using the Law of Demeter meant that my ControlPanel class would end up with a fat interface (Fat interfaces). I've decided to go with the Law of Demeter for two reasons. The first being that using Getters and setters to access things that are at deeper levels in my design, creates a lot of coupling between classes. This is explained below:
ControlPanel.getCurrentChannel().getSettings().setAmpModel("RectoDual");
This code lets the ControlPanel object directly modify a variable stored within the Settings object, so this makes the ControlPanel object become dependant on the internal structure of the Settings class. It is much better to use Law of Demeter because this means if the Settings class was changed, then only the Channel class would have to have code modified.
Secondly, Fat interfaces states that: "A fat interface is usually the one which is aimed at serving more than one set of clients." but this statement doesn't apply to the fat interface created by in the ControlPanel class by using the Law of Demeter. The reason being that the interface the ControlPanel class will have, although large, is the same interface that the real version of the Flextone provides and doesn't attempt to serve more than one set of clients. So in a way, having a ControlPanel class with a fat interface is sticking to the design maxim of Model the real world.
The most interesting thing that arose from this situation is that even though I chose to use the Law of Demeter, my code still has Getters and setters! This is interesting because the Law of Demeter and Getters and setters are supposed to be conflicting ideas, but I have happened to use both ideas in my design. I wanted to use Law of Demeter and Tell, Don't Ask because of the reasons stated above but what about Model the real world? Most of the actions performed when using the real-life version of the amplifier are simply setting different variables to have different values, e.g. setting the volume, setting the tempo of the delay and so on. Because of this, it makes sense to use Getters and setters in my design to allow the user to perform these actions. My design is using Law of Demeter but doesn't completely align with the idea of Tell, Don't Ask because my ControlPanel class has getter (asking) methods that simply tell the Channel class to do the asking for it. So in a sense, I managed to balance out conflicting forces.
Third design
My idea of implementing the second design in order to discover more things that could be improved worked quite well, and as mentioned above I struck upon a few conflicting forces during the implementation. Below is my third class diagram which shows the results of the implementation. Not too much has been changed in terms of new classes being made but Law of Demeter did help to clarify the behaviour of the design.
A critique of my third design
Overall, I believe that my third design is quite good because it encompasses all the core functionality that is provided to a user in the real-world version of the amplifier. You can select channels, select amp and cabinet models, select effects and save a group of settings to each channel. But there are a few things aren't quite right yet...
For example, my use of the Strategy design pattern works quite well to handle the switching between different types of effects. But it doesn't handle the situation where you aren't using any effects. This is where the Introduce Null Object refactoring technique is useful.
Another problem is that when selecting a new amp model, the design doesn't constrain you to only using the amp models it actually has. Currently it doesn't store a list of AmpModel objects that your amp selection is checked against. To further explain this problem, if you used the command "Select Amp Banana", the current design would simply create a new AmpModel object with the name "Banana". What the design should really do is check your selection against a pre-defined list of AmpModels. If this constraint was implemented then I could also implement some method called createNewAmpModel() or something similar that allowed you to add new amp models to the list.
Continuing on from the idea of being able to add new amp models, it makes sense that you should be able to have more than 4 different channels to choose from. Currently the ControlPanel constructor creates a new Hashtable and populates it with 4 channel objects. But I think it would be nice if you could dynamically add more channels as you are using the program instead of this option being hard-coded into the constructor. Maybe this calls for a Factory Method but there might be a different way of doing this as well.
Also, currently the main point of control is the ControlPanel class. But I think the name of this class should be changed if I implement functionality mentioned above because really it isn't the control panel that knows what amp models the amplifier has but instead is the amplifier itself. So I think Amp or Flextone might be a more suitable name for the class.
My final design
Below is the class diagram of my final version of the design
In terms of classes, I have only introduced two new classes. These are the NoDelay and NoEffect classes. By adding these two classes I was able to handle the situation where no delay or effects are in use. This was mentioned earlier in the critique of my third design. I took the idea of Introduce Null Object and modified it slightly to suit my design. The Introduce Null Object technique suggests creating an isNull() method in the superclass of the null object and over-riding this method in the null object itself. This means that checks for null in the code can be replaced with calls to isNull() on the objects being used. For my design this isNull() method was not necessary, I just needed a class to represent the concept of no effects or delay being used. The NoDelay and NoEffect methods still implement the processSignal() method but return different output stating that the signal is not being processed. This is the same idea as the signal bypassing the sound processor that would usually modify the signal to create the effect.
During the critique of my third attempt I talked about the idea of using the Factory Method design pattern to implement new functionality for creating new Channels, AmpModels and so on. Although the Factory Method is a useful design pattern I decided that it was not needed because it is designed to handle a problem I wasn't actually encountering. The Factory Method is very useful when a particular class (in my design it's the Amplifier class, formerly known as the ControlPanel) cannot anticipate the class of an object (a new Channel) it must create. In my design the Amplifier class does not need to anticipate what type of object it needs to create because that information is supplied by the user. Instead my final design simply creates a new Channels, AmpModels, CabinetModels and Normal/Delay Effects when the user instructs the program to do so, e.g with the command "Add New Channel". This decision is good because it's important to Do the simplest thing that could possibly work.
The main() method that I was using to handle user input was getting a bit large so I decided to use the Extract Method to clean up the code, this made a few new static methods but gave a good result. Even though the main method is still sizeable, I managed to remove about 200 lines of code. The code that is still there is mainly if-statements so it's repetitive but not too messy.
The only feature that doesn't fulfill it's potential is the showSettings() method. This method displays the settings held in the savedSettings object, so if a change is made to some settings, showSettings() won't display this until saveSettings() has been called. saveSettings() is called whenever a user types the command "Save Settings". Although this isn't optimal behaviour, generally a user would modify a large range of settings before saving and would be aware of what settings have been changed.
Overall, I think my final design has managed to capture the fundamental behaviour of the interface of the real Flextone III, and has also managed to extend this behaviour in a few ways with the ability to add new channels, amp models, cabinets and effects. Although the Amplifier class itself has a reasonably large interface, this is understandable due to the nature of the Flextones interface and the use of the Law of Demeter.
Source code
The .zip file below includes all source files for the design study, a read-me explaining the various commands, and the executable .jar file.
guitarAmplifierDesignStudy.zip