-
Notifications
You must be signed in to change notification settings - Fork 0
Machine shop second 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 #33. The method
inputData()(line 184) suffers from Long Method because it is an unnecessarily long method that does way too many different things. It could easily and should be broken up into different methods which in the process would make the code far more readable.
We will try to minimize the nested if/else statements and for loops using the Extract Method refactoring. We will then plan on making these extracted methods not take any methods to improve readability. Afterwards, we will try to extract anything leftover that is duplicated code.
This will most likely be the hardest refactoring we will have to do because of how large this method is. We will have to try to make sure the extracted methods have no arguments, and we will go into this very large method not knowing at all what it's supposed to do, so we won't know what will break or won't break. This will most likely take the most amount of time out of all of our refactorings.
This should have a large impact on the code due to the sheer size of the
inputData()method. I expect we should have at least 5 new methods after this large refactoring. It should improve the readability after this refactoring and a lot of these new methods can be moved to other classes as necessary.
This will resolve an issue not currently present in the issues page. After refactoring the
moveToNextMachine()method, there was leftover dataclumps present on lines 39 - 65. There's also some data clumps present in lines 36, 46, and 55 after we shortened thechangeState()method that can be extracted.
We plan on using the Extract Field, and Preserve Whole Object methods in order to condense these data clumps and reduce repeated code.
This should be very minimal in difficulty, as it's as easy as using Eclipse's built in Extract Object method. I don't imagine this will take too long either.
This should have a minimal impact on the code as there's minimal amounts of data clumps present in these methods.
This is to address the array base indices starting on 1 instead of 0. This particular refactoring focuses on the array of machines in MachineShopSimulator class. Typically most code have their array indices start at 0 so it would be better to make our code do the same.
Our plan is to first thoroughly look at the code to understand all the locations we would have to make changes. At the moment it seems like changes are needed in MachineShopSimulator and EventList. Then we probably make the changes all at once (since this code base isn't so big) and use the tests to validate the changes.
We believe it shouldn't be too hard. The time required on this task may vary though depending on if we change the code correctly.
First and foremost this should allow MachineShopSimulator to use array starting with 0 instead of one. Secondly this code base would adapt easier to new developers that often use array starting with zero base.
In the MachineShopSimulator class, we created a method
getMachineNumber()that searches the array of machines and returns an integer regarding a machine. We would like to get rid of this method call as soon as we can because this is an inefficient method once we have too many machines.
We plan to construct an integer field inside the Machine class. This allows each machine to know their number and also give their number to the other class.
This refactoring is not difficult at all and shouldn't take too long as all we have to do is construct a field and change code that calls for the integer.
In addition to making the code more cleaner this would greatly impact how efficient the code is. The method we currently have right now is not good at all as it has to traverse the array of machines to get the integer whereas it would be better for a machine to know its integer.
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.
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?
It was a great iteration indeed. We finished all of the proposed refactorings, and didn't really run into too many issues.
-
Shorten inputData() Method: This refactoring actually didn't end up being as hard as what was anticipated. The methods that were extracted were very easy to identify from the long method. We managed to separate it into 4 methods based off of the if/else blocks and the for loops. Overall, it didn't have a huge impact, but it made the inputData() method a lot easier to read. Since they were smaller methods, we were able to change the for loops into for-each loops since they are easier to read and shorten methods even more. Apart from that, there wasn't really anything else we saw that could be refactored after this reafactoring.
-
Cross Class Method Calls: This proposed refactoring wasn't very time consuming, but did help clean up the code a little bit. This produced several methods that we put into MachineShopSimulator() class because these methods were more appropriate for the main class (since it is the 'simulator'). This helped clean up the methods changeState() and getnewJob() that are present in the Machine() class. This didn't have a huge impact, but did make the above methods much easier to read. After doing this, we were able to start removing all of the get methods that we were able to, since we believe a class should not pass their parameters to other classes.
-
Array Base start at 0: This was another fairly quick refactoring. We mostly just went through and changed most of the index calls to use 0 instead of 1. We did run into some issues, but we troubleshooted using jUnit tests error statements to find out where the issue was. We also got Nic to help us troubleshoot and he helped us find the missing index call. This refactoring was very minimal, so not much was changed, but it was a necessary change since typically Java arrays start at 0 rather than 1. After this refactoring, we were able to use for-each loops instead of for loops in various methods.
-
Machine Object Lower Index Calls: We deleted the method getMachineNumber() and created a field in Machine class called MachineId to grab it instead of calling getMachineNumber() everytime in hopes to improve performance. This wasn't very difficult, we just had to go through after deleting getMachineNumber() and replace the index calls with a call to grabbing a Machine's ID number. This had a significant impact on the code since it removed many calls to grabbing indexes and it looks alot cleaner to instead grab the MachineID for each machine. We didn't notice anything we could refactor after this change that didn't already need to be refactored.
-
Comments and Naming Scheme: As the name implies, we just went through each class and changed names where necessary. We also removed unnecessary variables and comments by cleaning up bits of code to make it more readable so that a comment or variable might not be necessary. The most difficult part of this was figuring out what the code actually does in order to properly rename something and decide if a comment is necessary or not. Overall, it didn't take a huge chunk of time. It did have a pretty big impact on the code however and really put a nice finishing touch on the entire code base. This did open doors for us as we went along to clean up parts of code to make it more readable and put some variables in classes that made more sense.