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 efforts on:
- review how your APIs have changed during the course of the project
- find and fix design issues in your code
- ensure that your core code follows SOLID design principles and supports adding new features and hiding implementation decisions
API Changes
Start by reviewing your original external APIs (in your DESIGN_PLAN.md
file and the interfaces you created to express that design) as well as your current APIs. When looking at the current version of your API you may decide that some methods do not actually need to be public or that a new interface or class would be useful to more clearly separate part of an internal from an external API or to hide an implementation choice or to add flexibility to the code.
Recall that one of the requirements for this project is that your team complete a document, doc/API_CHANGES.md
, that describes how your APIs have evolved over the course of the project. This exercise is intended to help you determine the content of that document. Thus, as a team, your goal is to determine:
- what changes that have been made to the APIs
- if those changes are major or minor (justify your distinction based on how much they affected your team mate's code)
- if those changes are for better or worse (if for the worse, is there a way to improve it or was the original API overly optimistic)
- if your foresee any significant changes coming in the next few days as you finish the project (based on your experience and the fact that you now know all the features to be implemented)
Record the results of your discussion in your team's doc/API_CHANGES.md
file on your master branch. Include any additional supporting documents you feel are useful as well.
Refactoring
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 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.
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 discussion files (doc/API_CHANGES.md
and doc/REFACTORING_DISCUSSION.md
) to the team's repository. Make sure everyone's NetIDs are in the title of your Merge Request.