Static 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 the static analysis tool SonarQube, focus your refactoring efforts on:
- removing duplicated code
- fixing design checklist issues in your code
- improving your use of standard Java idioms to improve your design
SonarQube combines a number of static code analysis tools to report a variety of statistics and issues regarding your code. This analysis is performed by automatically looking for certain features in your code (such as that no instance variables are public). We have matched the general issues to the course'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 will need to add two files to your project so that this analysis is done automatically when code is pushed to the master
branch. You are not required to remove all issues found by this tool, 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 the report.
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. One of the tabs on the Checklist Report is Duplicated Code, which shows a side by side comparison of places in your project that contain duplicated code.
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.
Checklist Refactoring
Three "tabs" organizes the variety of possible coding issues into the higher level design goals given in the project's Design Checklist. Some items on the checklist may not have any issues (green — YAY) and some may have a lot of issues (red — BOO). Your team should focus on the items with many issues and decide how best to refactor the code to fix them.
In your discussion file, describe why you chose the fixes you did and what, if any, alternatives you considered.
General Refactoring
Two "tabs" organizes the variety of possible coding issues into the categories Code Smells or Java Notes. Code Smells are based on those items we have studied earlier in the semester. Java Notes are based on Joshua Bloch's Effective Java book which describes typical issues in Java code that should be fixed in a standard idiomatic way. As usual, some of these may have easy fixes and some may require more thought. Your team should discuss how best to refactor the code to fix each one.
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 both people's NetIDs are in the title of your Merge Request.