NicMcPhee/CraftBukkit — pull request #3 by MaxMagnuson (2014-04-19 19:17:49+00:00)
Title: [C] Extracted logic into its own class from method d in BlockSapling, Fixes My Academic Knowledge

The Issue:
Method d in class BlockSapling contained multiple code smells. One of the smells was duplicated logic which could be found in the different switch cases. Another smell that d contained is that it was a very long method containing logic for multiple functions. We believe this is because many local variables were instantiated within the class that were needed for purposes other than its original purpose. This caused the method to be bloated with multiple functions.

Justification for this PR:
The smells outlined above made the method's logic difficult to understand and made the method unnecessarily complex. It also made the logic unreachable by other parts of the code. We felt that these issues were important to fix from both a code readability standpoint and a structure standpoint.

PR Breakdown:
To solve the issues above we created a new class, SaplingHelper, and extracted key variables and logic into it. First we took all of the local variables in d and put them in SaplingHelper as fields. This allowed us to extract the duplicate logic in the switch statements condensing the logic into three methods in SaplingHelper. We then pulled out excess logic from d into SaplingHelper in the form of multiple methods. Thus focusing the scope of d.

Testing Results and Materials:
The code built successfully.

Note:
We ommited a ticket number because this was purely a refactoring
exercise for academic purposes, and we do not want it to make it back to
bukkit since we altered code in the server part. Also lack of testing
makes us question our change's correctness.