-
Notifications
You must be signed in to change notification settings - Fork 0
Machine shop first iteration proposal
Below are our prioritized list of refactorings. Our initial plan is to tackle these in this order, but we may need to revise that plan as we move into the refactorings.
This will address Issue #9. Since the class
MachineShopSimulator()is so large and contains several classes, it would be best to extract many of the subclasses and classes present inMachineShopSimulator()class to remove complexity and will make it much easier to refactor as we go along.
Our plan is to go through
MachineShopSimulator()class and look through each class/subclass present and see what we can move. We will move one at a time using the Extract Class method and Extract Subclass method. After that, we will fix the private modifiers using getClass methods for each class.
This shouldn't pose too much of a challenge for the most part, but it will be time consuming going through each class and subclass and moving them. The biggest challenge present will be making sure each class can read each method as needed due to all of the private modifiers.
This will remove the Large Class smell and will make the code much more readable because of the large amount of organizing and should be the first step before we do any other refactoring proposals due to the higher priority. This should also help with our next refactoring for Shotgun Surgery because it will be much easier to see what's dependent on where. We suspect this should have a large impact on the code because many classes are being made and moved around.
This will address Issue #22. After creating separate classes, the MachineShopSimulator has many methods that may not belong to it, and should be moved to their respective classes. For example, the method
moveToNextMachine()suffers from Feature Envy because it spends much of its time getting and modifying data from an instance of theJobclass. This suggests that much of that logic more properly belongs in theJobclass instead of in theMachineShopSimulatorclass.
Our plan is to initially extract one or more methods from
MachineShopSimulator()that isolate the parts of the logic that are particularly tightly tied to each class. We'll then move those methods into their respective classes, making sure to provide any necessary data access back toMachineShopSimulatoras we go. We can use the Move Method and the Extract Method to accomplish this.
The Eclipse refactoring menu will help automate both the method extraction and the method movement steps, so we expect the biggest challenge to be identifying method extractions that nicely set the state for the method movement step. It seems likely that there will be a certainly amount of trial-and-error in this part, especially since we're not very familiar with this code.
This helps address both this Feature Envy smell, but also the Data Class smell (where the
Jobclass holds data but has no logic), and hopefully opens the door to moving other logic intoJob. We suspect that this refactoring on its own will probably have a moderate (not small, but not huge) impact, but we hope that the sequence of refactorings that this kicks off will improve the clarity of many individual methods and the greatly improve the overall organization of the project by moving logic to more sensible locations.
This will address Issue #31. Many of the the classes/ideas/methods are hopelessly intermingled within one another making it very hard to make one change in just one location to change the functionality of the code. It also makes in very hard to follow. A simple example is the
inputData()method where calls are made to many points of data which are not near the method itself.
Our plan is to go through each new class we made out of
MachineShopSimulatorand see what fields they have. We should be able to use the Encapsulate method for each field and create getter and setter methods.
This should not be too hard since all we have to do is use Eclipse's encapsulate tool to do it. Then we remove unnecessary setter or getter methods by hand.
This should also help with our next refactoring for Shorten Methods because it will be much easier be able to move methods around and extract chunks of code from these absurdly long methods. We suspect this shouldn't have a large impact on the code.
This will address Issue #19. There are three methods in the
MachineShopSimulator()that have nested if and else statements and for loops that can be refactored to smaller and separate methods. These methods arechangeState(),inputData(), andmoveToNextMachine().
First, we will need to make more tests to ensure that the methods work properly after extracting methods within these longer methods. Then, once the tests work and pass, we can go through and extract the nested for and if/else parts of each method one at a time. We can accomplish this using the Extract Method, Replace Temp with Query, Introduce Parameter Object, and Preserve Whole Object methods to reduce the long methods’ complexity.
The Eclipse refactoring menu will help automate both the method extraction and the method movement steps, so we expect the biggest challenge to be identifying method extractions that nicely set the state for the method movement step. It seems likely that there will be a certainly amount of trial-and-error in this part, especially since we're not very familiar with this code.
This helps address the Large Methods present in
MachineShopSimulator. This should have a moderate change to the code and will certainly help make it more readable in the long run and reduce any complexity tied to those methods we will be minimizing.
This will address Issue #14 and Issue #28. Many of the names in this file don’t specify exactly what they are supposed to do or what their purpose is. There are also just some rather confusing names that don’t read well with the code and some abnormally long variable names. After the names are changed appropriately, there should be no reason for the comments to exist because the new names should be able to spell out what the code is doing.
First, we need to change many of the variable/method names for each class so that they make sense in the context of what they are meant for or what they will accomplish. After that, we should be able to go through and remove most of the comments because comments aren't necessary if the names make sense with the code. If comments do still exist, then the code should be it’s own method and we can use Extract Method to remove the comments.
The Eclipse refactoring menu will help automate both the method extraction and the method movement steps for if and when we need to remove comments. The biggest challenge will be coming up with names for almost everything here, so it will pose a longish task to accomplish and figuring out how to make the code readable enough to get rid of comments.
This will make the code much more readable and should have a pretty big impact on the code, assuming we are able to come up with good enough names and can clean up some parts of the code enough so that it makes sense and is readable enough to remove the comments.
Just some stuff we would like to get to if there is time, if not, it's not the worst thing ever to miss out on it.
- More tests
- Fix arrays to start from 0 instead of 1
For each refactoring you did, summarize the outcome. How difficult did the refactoring turn out to be? What was its overall impact? Are there new doors that this refactoring opened?
Overall, most of our refactorings weren't too difficult. We completed the Extract classes, Encapsulate Fields, Feature Envy, and part of the Shorten Methods refactorings. For Extract Classes, we made Machine, Task, Job, and EventList classes. For Encapsulate Fields, we made all of the fields private and removed unnecessary getters and setter methods. For Feature Envy, we moved changeState into Machine, and moveToNextMachine was moved to the Job class. Lastly, for the Shorten Methods refactoring, we extracted a method from changeState method and cleaned up the method in various places. We also extracted 2 methods from moveToNextMachine and also cleaned up the method in various places.
We still need to finish Shorten Methods refactoring for the inputData Method that's present in MachineShopSimulator class and fix the naming schemes for pretty much the entirety of the code, and remove unnecessary comments. Naming scheme will most likely be the last thing we do since we still don't entirely understand the code.
For new things we can do, we can change the Array structure to start at 0 instead of 1, and the MachineShopSimulator.elist... that's present in some methods can be extracted. We can also add an integer field to Machine to get rid of the many calls for getting indices.