Checklist Code Review and Refactoring

As a team, use today's lab time to focus only on the project's design rather than adding new features. Using static analysis to highlight some of the project's Design Checklist issues, focus your refactoring efforts on:

• removing duplicated code
• reducing the size of your longest methods
• fixing Design Checklist issues in your code
• adding abstractions to help make parts of your design more flexible

Reports were generated automatically by looking for certain features in your code (such as public instance variables). General issues are organized in this project's Design Checklist to clarify why these issues are relevant to your design. It should be noted that this analysis in no way guarantees your code is well designed, it only helps find obvious potential flaws and brings them to your attention. For example, it is an obvious design flaw to have public instance variables, but you can still have a bad design even if all of your instance variables are private.

You are not required to remove all issues found in these reports, but you should be prepared to justify why you left any issues in your code. This is best done in comments near the lines flagged as issues in any of the reports.

To work on today's exercise, create a branch in your team's repository called refactoring_NETIDs, where NETIDs is the Duke NetIDs of the people on your team participating in the exercise. You will refactor some code and describe your decision making process in a separate file, called doc/REFACTORING_DISCUSSION_NETIDs.md, using Gitlab's markdown format.

Before starting today's exercise make sure everyone has committed their latest changes on their branches just in case so no work is lost. At the end of the lab, you will create a Merge Request so the changes must approved by the team before being integrated.

Duplication Refactoring

You should know by now that code duplication is number one in the Code Smell stink parade. Still, as things need to get done, you rush and copy code anyway or you may not realize that similar code was written in different places in the project.

We ran Google's long abandoned tool CodePro AnalytiX on each project to show a side by side comparison of places in your project that contain duplicated code and made the results available here.

Finding duplicated code is easy (so easy a computer can do it :), but refactoring it can take some thought. Sometimes the refactoring is as simple as creating a new method, sometimes it requires creating an inheritance hierarchy, sometimes a new class or abstraction is required. Discuss the options as a team and decide how best to refactor the chosen code section to remove the duplication.

In your discussion file, describe why you chose the fixes you did and what, if any, alternatives you considered. Use SOLID Design Principles to justify your larger refactoring efforts.

Long Methods

Long methods are another Code Smell that can reveal more than just centralization of your code — they are often also the source of inflexibility in your code because they are a sign of an object doing too much. By looking beyond simply breaking these methods up into smaller pieces, you may find violations of the Single Responsibility Principle or hard coded assumptions that can be reversed by creating an abstraction to support the Dependency Inversion Principle.

Looking at these methods exposes you to some pretty bad code in your Cell Society project :( After acknowledging that it is really part of your project, try to come up with ways to actually improve the current design together (rather than blaming people that participated in writing the code) because these are often the best places to start to find significant design flaws because long methods can be caused by almost any design problem (and most often by many at once!) but especially by failures of abstraction that are being handled by multiple conditional statements (with or without hard-coded magic values).

Using these Checklist Report results for each team, go to the 10 Longest Methods section and review the top three longest methods using the following steps and record the results in your discussion file:

1. List as many design issues as you can in the method (using line numbers to identify the exact code) from large things like (potential) duplicated code or if statements that should be generalized through polymorphism, data structures, or resource files down to medium sized things like poor error handling or long lambdas methods to small things like consistent coding conventions or ignored assignment design requirements (like using Resources instead of magic values). For many of these methods, this should be a long list of issues!
2. Organize the list of issues based on things that could be fixed together based on a different design choice or using similar refactorings and prioritize these groups based on which would provide the most improvement to the overall code (not just this method).
3. Describe specific overall design changes or refactorings to fix the three most important issues you identified.

After this discussion, make the chosen refactorings to reduce the length of these methods.

Checklist Refactoring

Again, using these Checklist Report results for each team, prioritize the remaining issues and refactor the team's code (it does not necessarily have to be code you wrote) to fix the issue and improve the overall design of the project.

In your discussion file, describe why you chose the fixes you did and what, if any, alternatives you considered.

General Refactoring

Given all the code flaws you have looked at, consider the larger design context and how abstractions (Java interfaces or superclasses) can be used to improve the overall design of your project. Create an abstraction and refactor your code to use it instead of hardcoded values associated with concrete classes.

In your discussion file, describe why you chose the fixes you did and what, if any, alternatives you considered.

Submission

At the end of class, use Gitlab's Merge Request to submit your group's refactored code and your doc/REFACTORING_DISCUSSION_NETIDs.md file to each team's repository. Make sure everyone's NetIDs are in the title of your Merge Request.