Iteration 2 Proposal - RenjiClark/Refactoring-SDD-2010 GitHub Wiki
Prioritized list of refactorings
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.
Refactoring 0 Privatize as many methods as possible
Not all methods should be public
We need to make as many methods private since not all of the methods need to be publicly available and should be made private.
Planned refactoring
Get rid of as many public modifiers to the methods as possible and make them private. Then go through class by class and fix the errors by making a call to the method that was privatized.
Technical difficulty
Almost every method is public and it will definitely take a while to make them private. Not only that, but there's also a lot of dependencies to deal with so it's hard to mess with many of the methods. Could pose quite the difficulty.
Impact on the code
This should just make all of the methods private, however, it will also create a lot of methods for getting the methods to call them in different classes and will most likely add a lot of new things to the code, especially since there is so many methods to privatize.
✓ Refactoring 1 Shorten the returnPoints() method in PointsTable
Long Method returnPoints()
The returnPoints() method is very long and kind of unreadable. If we shorten down the method to 2 or 3 methods and shorten some statements, it can improve the readability greatly.
Planned refactoring
What refactoring (preferably something with a name from one of the texts) do you plan to apply to the problem? For example:
Use Eclipse's "Extract Method" method to shorten the returnPoints() down to 2 or 3 methods.
Technical difficulty
How technically difficult do you expect this refactoring to be? For example:
Shortening the method shouldn't be too hard, however, I'm worried it might be more difficult to try to resolve the dependencies for the main method for returnPoints since there are a lot of dependencies throughout the code and it is more than likely that returnPoints is used in many places.
Impact on the code
How large an impact do you expect this refactoring to have on improving the code base? For example:
Not too much that will change, it should just make the returnPoints method more readable than what it was before and make a couple of new methods to go along with that.
✓ Refactoring 2 Delete awt itemeventtest.policy
See Refactoring 5. This still exists to compensate for a commit message.
✓ Refactoring 3 Formatting
Improper formatting in several files
To improve readability, it is important to remain consistent with the same code formatting of Java. Several places throughout the code, there is formatting, such as unnecessary spaces or curly braces on the same line as methods, that need to be fixed.
Planned refactoring
What refactoring (preferably something with a name from one of the texts) do you plan to apply to the problem? For example:
Just go through every line of code in each file and check what needs to be fixed and moved around and formatted.
Technical difficulty
How technically difficult do you expect this refactoring to be? For example:
This will not be a difficult refactoring. Might take a while, but it should be very easy.
Impact on the code
How large an impact do you expect this refactoring to have on improving the code base? For example:
Depending on how much formatting needs to be fixed, this can end up being a pretty big refactoring, or not. Either way, it will definitely improve readability of the code.
✓ Refactoring 4 Remove an input from addPanels
See Refactoring 5. This still exists to compensate for a commit message.
✓ Refactoring 5 Minor Edits
Issue
This is a catch-all for all tiny refactorings that are neither important enough nor sweeping enough to really change the code outside of a few lines.
Planned refactoring
Below is the list of tiny refactorings done:
-
✓ Delete test.policy
-
✓ Remove an input from ColorSchemeController.addPanels()
-
✓ Condense logic in ColorSchemeController.itemStateChanged()
-
✓ Maybe remove RunController.extractPath(), but only do so if understood why it exists in the first place.
Technical difficulty
Not difficult
Impact on the code
In total this will be a large change on the code, but each individual change is tiny or trivial.
Refactoring 6 Shorten Try Catch Block in RunController
Large try catch block in RunController file
The large try catch block can be shortened in RunController. It's not entirely unreadable, it's just not very pretty having a large block of code of tenish catches in the try block. These can be made into individual error methods.
Planned refactoring
What refactoring (preferably something with a name from one of the texts) do you plan to apply to the problem? For example:
We plan on extracting each catch block into individual methods throwing errors instead of having all of these catches. Hopefully, we can be able to completely remove the try catch block with lots of methods, but we may need a throw somewhere to make it run.
Technical difficulty
How technically difficult do you expect this refactoring to be? For example:
This could be easy, but probably not. Messing around with try catch blocks is never fun, especially when you don't know how it works, but it might not be the worst thing ever and could just be as simple as extracting each catch into a method.
Impact on the code
How large an impact do you expect this refactoring to have on improving the code base? For example:
This should remove the entire try catch block in the RunController class and replace it with several methods that throw errors.
Outcomes
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?
Refactoring 0
This was probably the one impossible refactoring. After attempting it a few more times, we were unable to actually get any of the methods to be privatized because there's so many packages and so many dependencies via tests and every single public method is used somewhere else where it needs to be public. So, this refactoring was unfortunately a fail, but we tried and that's what counts. So, no change had happened on this part.
Refactoring 1
This was extremely messy code. We managed to clean returnPoints() up just a bit though by extracting a couple of long lines to a couple of methods. This wasn't too difficult and it looks a lot better, but still isn't the prettiest. Had we had more time, we probably could have gotten to this one and worked on it more to make it even better, but ran out of time unfortunately. This refactoring simply made a method look better, so improved readability was obviously the change for this refactoring. It was not a huge change but it did improve the readability of the returnPoints() method by a lot.
One cool idea for this one was to make a whole new class for auto generating a lot of the html present throughout the code using some Java tools that are out there, but this would have been a month long project that we just didn't have the time to try to implement unfortunately.
Refactoring 2
This one ended up being too small to be a "real" refactoring so we decided to move it into Refactoring 5 since it was so small. It was extremely easy and we had no problem removing it. It simply improved readability and got rid of unnecessary content.
Refactoring 3
This was not the most difficult of refactorings, but it was a very widespread problem. It did significantly improve the readability of the code.
For example, we changed stuff like this
if(boolean){codecodecode;}
else{morecodecodecode;}
to the more sensible format
if (boolean) {
codecodecode;
} else {
morecodecodecode;
}
There were many other cases of } just floating at the end of lines.
Refactoring 4
This was another refactoring that we originally thought could have been a full refactoring that ended up being extremely easy and moved over to Refactoring 5 due to how easy this one was. This simply removed a call in ColorSchemeController to help improve readability since the input was unnecessary in addPanels(). This improved readability and helped out by getting rid of unnecessary code.
###Refactoring 5
Delete test.policy No code changed other than the removal of a useless file
Remove an input from ColorSchemeController.addPanels() One call was changed, readability went up
Condense logic in ColorSchemeController.itemStateChanged() Readability was improved
After looking further, we decided the extractPath() method was actually necessary and we were able to use it a few more places.
###Refactoring 6 We deemed this was a refactoring was not worth doing, as the code block, while weird looking, is as readable as it will ever be. Had we had more time, we might have gone back to try to fix it by making each of the catches into a method that throws error messages, but that would have been a lot more code and might improve readability, it would be very funky to deal with that wasn't worth our time we decided. So, no change for this refactoring.