Apply design pattern to some classes - #29
Open
terry2511 wants to merge 64 commits into
Open
Conversation
Refactoring Operation - Introduce Explaining Variable - Extract Method Refactoring target - updateCameraForRoamingMovements(TimeStep timeStep) For readability To remove duplicated codes
Target : Tri class in seventh.math package Reason : An inheritance should from an IS-A relationship, but tri(if i guessed correctly, the meaning of this is triple) is not a pair.
Refactoring Operation: - Replace Magic Number with Symbolic Constant Refactoring Target: - the number of array of Button and Label because the numeric value(7) had no obvious meaning, I replace the value with constant
Refactoring Operation: - Extract Method Refactoring Target: - createUI method I think it needs to isolate independent parts of code.
createTitleLabel -> setupTitleLabel createCancelButton -> setupCancelButton
Refactoring IS-A Relationship
ProgressBarView I extracted the source code that needs explanation.
Refactoring Operation: - Extract Method Refactoring Target: - refreshButtons method 'refresh' contains 'initializing' and 'setup'. it is needed to separate these two different functions.
Refactoring Operation: - Extract Method Refactoring Target: - refreshButtons method - setupWeaponClasses method to remove duplicated codes ease to change the weapon types
Refactoring operation: -Replace Data Value with Object -Form Template Method Refactoring target: -getClassDescription method to reduce duplicated codes if new descriptions of weapons are added, you can simply add a new subclass without touching the existing code
2.ClienteGame.java
public void applyGameUpdate()
Game.java
public void update()
public boolean playerSwitchedTeam()
PlayerAwardSystem.java
public void roundEnded()
public void addKill()
3.too many functions, duplicated codes in one method
Target : ClientMain class in seventh Reason : I extracted the source codes by function.
Refactoring Operation: Extract Method (contactDedicatedServer method) Refactoring Target: init method in GameServer class of seventh.server package Reason: to satisfy SRP 2. Refactoring Operation: Extract Method (printHeader,pringBody,printFooter method) Refactoring Target: dump method in BitPacker class of harenet package Reason: divide dump into 3 parts to satisfy SRP printFooter was duplicate code (dump,dumpBytes) 3. Refactoring Operation: Extract Method (removeDisconnectedPeer) Refactoring Target: disconnect method in Host class of harenet package Reason: decouple removing peer from disconnect to satisfy SRP
2.FastMath.java static public final int random() static public final float random() public static float a_sqrt public static float a_isqrt 3.placing the result of an expression in a local variable for later use in the code.
Extract Method
Refactoring Seventh.game.type.obj.ObjectiveScript.java
Refactoring WeaponClassDialog, CameraController classes
2.ClienteGame.java
public void applyGameUpdate()
Game.java
public void update()
public boolean playerSwitchedTeam()
PlayerAwardSystem.java
public void roundEnded()
public void addKill()
3.too many functions, duplicated codes in one method
1.Replace Temp with Query
2.FastMath.java
static public final int random()
static public final float random()
public static float a_sqrt
public static float a_isqrt
3.placing the result of an expression in a local variable for later use
in the code.
Target Class : seventh.shared Arrays class Reason : Arrays class implement quicksort and using it.
1. Name of applied design pattern - Template Method Pattern 2. target - methods in World class getNorthZone getNorthWestZone getNorthEastZone getEastZone getSouthZone getSouthWestZone getSouthEastZone getWestZone 3. reason - to remove duplicated codes.
1. Name of applied design pattern - Abstract Factory Pattern 2. target - add TeamStrategyFactory abstract class, DefaultAISystemTeamStrategyFactory class. - delete switch context in init(GameInfo) method in DefaultAISystem class and add factory context. 3. reason - make it easy to add another AISystem and maintain AIStrategy codes.
Apply a design pattern
seventh.shared.Arrays class separate implementation of sorting method from Arrays
Target : seventh.shared.Arrays class Reason : Factory Method pattern can create objects with common ancestor
Target : seventh.shared.Arrays and, seventh.shared.SortStrategyFactory Reason : Two more factories are not necessary. Only one factory is sufficient.
Starategy Pattern
Target : seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAction Reason : Separate implementation of start and getAction method from AIGroupAction Class for avoiding duplication
Target : seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAttackAction seventh/ai/basic/group/AIGroupDefendAction seventh/ai/basic/group/AttackGetAction seventh/ai/basic/group/AttackStart seventh/ai/basic/group/DefendGetAction seventh/ai/basic/group/DefendStart seventh/ai/basic/group/GetAction seventh/ai/basic/group/Start Reason : Separate implementation of start and getAction method from AIGroupAction Class for avoiding duplication
Target : seventh/ai/basic/group/AIGroupAction seventh/ai/basic/group/AIGroupAttackAction seventh/ai/basic/group/AIGroupDefendAction seventh/ai/basic/group/AttackGetAction seventh/ai/basic/group/AttackStart seventh/ai/basic/group/DefendGetAction seventh/ai/basic/group/DefendStart seventh/ai/basic/group/GetAction seventh/ai/basic/group/Start Reason : Separate implementation of start and getAction method from AIGroupAction Class for avoiding duplication
Target :
seventh/ai/basic/FeelSensor
seventh/ai/basic/Sensors
seventh/ai/basic/SightSensor
seventh/ai/basic/SoundSensor
seventh/ai/basic/SensorFactory
Reason : For useful skill to prepare for changes in objects
template method pattern strategy pattern
Design Pattern: Factory Method Pattern
ai/basic/memory
Apply design patterns - Observer, Template Method, Factory Method
Owner
|
Please fix formatting issues (mainly tab -> 4 spaces). |
tab -> 4 spaces
Change tab to 4 spaces
tonysparks
reviewed
Jun 12, 2018
| } | ||
| System.out.println(); | ||
| System.out.println("+--------------- ------------- ------- ------ --- -- -- - -- -- --"); | ||
| StatementFactory statementFactory = new StatementFactory(); |
Owner
There was a problem hiding this comment.
This is a lot of extra code for not a lot of benefit. You want to create Factories when there a lot of different types or configurations.
This also adds in an extra layers of complexity -- instead of reading the code inlined, the reader has to now understand StatementFactory, Statement and the concrete classes of BitStatement and ByteStatement (along with grappling the inheritance).
|
|
||
| import seventh.shared.TimeStep; | ||
|
|
||
| public class SensorFactory { |
Owner
There was a problem hiding this comment.
Very similar to the Statement comment -- I don't see the added benefit of SensorFactory. The Sensors class already houses all available Sensors -- which negates the need for this class.
| int adjacentZoneY = bounds.y - (bounds.height/2 + fuzzy + minDistance); | ||
| Zone adjacentZone = getZone(adjacentZoneX, adjacentZoneY); | ||
| return adjacentZone; | ||
| return new NorthZone().getAdjacentZone(bounds, fuzzy, minDistance); |
Owner
There was a problem hiding this comment.
I agree, breaking the code up into separate classes makes the code more readable. However, this does create more objects (and these methods are called frequently any given game frame) which creates a lot of garbage on the heap -- which in turn causes the Garbage Collector to do more work (which increases the game pauses).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We apply design pattern to some classes.