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.

You probably want to remove all my italicized "instructional" text as you work through the proposal.

Make sure to commit frequently as you work. At a minimum you should commit after completing each refactoring so you can document that accomplishment in your commit message. For more complex refactorings you'll probably want/need to commit multiple times as you go.

Refactoring 0: Duplicate Code

Issue/bad smell being addressed

There is a lot of code duplication throughout the code. Specifically, we plan on removing code duplication in the following methods: runTaskTimer() and runTaskTimerAsynchronously() methods in the CraftScheduler.java class and contains() and remove() methods in the LongHashSet.java class. The code duplication for CraftScheduler.java is on lines: 120-127 and 137-144. The code duplication for LongHashSet.java is on lines: 63 to 73 and 132 to 142.

This will address issues 1 & 2 from the Issues wiki page.

Planned refactoring

In order to remove the code duplication, we plan to use the tools provided by Eclipse along with some ingenuity to extract the duplicate code into separate methods to be used by the methods containing the duplicate code. We will use Eclipse's "Extract Method" refactoring tool to do so.

Technical difficulty

Given that the duplicate code is within their respective classes, we don't envision it being too difficult to simply extract the method into it's own method and have it used in their classes since Eclipse's tools work very well that way. There is only 7 lines of concurrent code duplication in CraftScheduler.java class and 10 lines for longHashSet.java class, so it's not too much code and shouldn't cause many issues.

Impact on the code

These two refactorings should help improve the readability and also lower the number of lines of code present in the methods and in the class itself.

Refactoring 1: Dead Code

Issue/bad smell being addressed

There is a large chunk of dead code in the sendChunkChange() method. This does not need to exist and should be removed from this world forever. The dead code is on lines 397 to 422 of the CraftPlayer.java class

This will address issue 3 from the Issues wiki page.

Planned refactoring

We plan to highlight the dead code, and press either the backspace or delete button.

Technical difficulty

Depending on how lethargic we get, it might take a lot of effort to actually go through the work to highlight all that code and reach our sweaty nubs we call fingers over to the edge of the keyboard and press either the backspace or the delete buttons. But we don't imagine that happening and should be a very easy refactoring.

Impact on the code

This will reduce the number of lines of code in the file and help improve readability. This will also shorten the sendChunkChange() method by a lot.

---

Refactoring 2: Long Method

Issue/bad smell being addressed

There is a 55 line long method present in the CreateEventFactory.java class called handleEntityDamageEvent(). This method contains large elseif blocks as well as some nested if blocks that can be extracted. This is far too long for a method and really should be reduced in order to help readability and reduce complexity. The method is located on lines 423 to 441

This will address issue 4 from the Issues wiki page.

Planned refactoring

We plan to extract the inner if blocks as well as the large elseif blocks into separate methods. We can probably get 2 extracted methods out of doing this. We will use Eclipse's built in refactoring tool to extract the inner blocks of code into methods.

Technical difficulty

Given that we are only doing 2 helper methods for this and that the 2 chunks to extract seem pretty obvious, we don't imagine this should be too difficult, depending of course on how many dependencies there are and how much this part was actually tested.

Impact on the code

This should drastically reduce the complexity as well as improve the readability for the handleEntityDamageEvent() method.

---

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 From this refactoring, we were able to eliminate the code duplication for each of the classes. In the CraftScheduler.java class, we took the duplicate code and made it into two methods: setDelay and setPeriod and used those for the runTaskTimer() and the runTaskTimerAsynchronously() methods. This was not too difficult, but we couldn't simply use Eclipse's built in methods and just used our own ingenuity to do so.

For the other class, LongHashSet.java class, we were simply able to use Eclipse's built in tools to refactor the chunk of duplicate code into a new method for each of the methods with code duplication: contains() and remove()

##Refactoring 1 This was a very easy refactoring and we were able to push backspace key successfully after highlighting all of that code with the mouse. We reran the tests and they passed. Go team.

##Refactoring 2 handleEntityDamageEvent was originally a 55 line method and we were able to reduce it to a 32 line method. It is a lot more readable and much less complex compared to what it was before. We were able to extract the large elseif block into setCause() helper method located on line 439, and the nested if block into another helper method now located on line 429. Again, this was not too difficult thanks to Eclipse being very helpful with its refactoring tools. Thanks Eclipse.