Lab Exercise : Static Code Review and Refactoring

In your Cell Society team, use today's lab time to focus only on the project's design rather than adding new features.

You will know your design is on the right track if your code:

• refers to abstractions more than concrete classes directly (especially abstractions your team created)
• relies on polymorphism when calling methods rather than conditional logic based on a concrete class's "type"
• uses subclasses to make it clear and easy to determine where to write varying features

Work on today's lab following these steps:

1. Individually, before starting, make sure you have committed your latest changes so there is no possibility any work is lost
2. As a team, discuss the primary issues found by the static analysis tool and how to approach refactoring the code to address them:
• Add everyone's names and NetIDs at the top of the REFACTORING_DISCUSSION.md file and add it to the repository in the doc folder
• Note your ideas, reasoning, and main conclusions using Gitlab's Markdown format
3. As a group, collaboratively refactor your team's highest priority issues
   • Create a new branch for today's work in your team's repository called refactoringLab

Submission

At the end of lab, use Gitlab's Merge Request to submit your team's updated code and doc/REFACTORING_DISCUSSION.md file from its separate refactoringLab branch to the team repository's master branch. Make sure the Title of your Merge Request is of the form "lab_refactoring - participating NETIDs".

It is fine to accept this Merge Request after lab whenever your team is ready.

Discussion

Use our app that helps organize and prioritize issues to clarify issues found using static analysis. It should be noted that static analysis in no way guarantees your code is well designed, it only helps find potential flaws and brings them to your attention. For example, it is clearly a design flaw to have public instance variables, but you can still have a bad design even if all of your instance variables are private. While you are not required to remove all issues found by this tool, you should be prepared to justify your decisions if significant issues are left in your code (e.g., with comments near the lines flagged as issues in any of the reports).

To begin discussing the code's design, review the top issues found in your code's most recent master branch using the following steps and record the results in your discussion file:

1. List as many design issues as you can in the method associated with the issue (using line numbers to identify the exact code) from large things like (potential) duplicated code or if statements that should be generalized through polymorphism or data structures down to medium sized things like code structure or unwanted public variables or methods to small things like magic values or inconsistent code.
2. Organize this 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 five most important issues you identified. Note how each change will improve the overall code design, what external changes others will have to make, and what, if any, alternatives you considered. Use Design Principles and Code Smells to justify the goals of your refactoring efforts.

Some issues may refer to the same piece of code (likely making that code a higher priority to refactor), some issues may have easy fixes, and some may require more thought or require more sweeping changes. For example, as we have discussed in class, sometimes removing duplicated code is as simple as creating a new method and sometimes it requires creating a new class or abstraction.

Additionally, consider the priorities of the project specific design goals minimally expected of every team:

• Encapsulation. Hide grid data structure's implementation:
  • NO public instance variables (also note, protected instance variables also break encapsulation and should be avoided, but public ones are simply poor design)
  • NO explicit public references to grid's data structure implementation (i.e., as a 2D array or a List of Lists) should appear as a parameter or return type
    Note, ideally, your grid class should implement an immutable interface that is used by the view classes to display the results of the simulation
• Abstraction. Support Open/Closed principle by creating effective inheritance hierarchies that have generally usable methods:
  • NO downcasts to specific subclasses or instanceof checks (except in equals() methods)
• NO chains of conditionals (i.e., if or switch) based on a "type" or enumerable property that distinguishes small differences in behavior
  
• MVC. The model should not depend on the view:
  • NO references (i.e., import javafx.XXX) to OpenJFX classes within your model classes (even javafx.scene.paint.Color)
  • ALL text displayed in the GUI, from buttons to menus to sliders to error messages, must be keyed from resource properties files using String.format() for dynamic information rather than '+'
    Note, ideally, even your property files should be separated into appropriate packages to further clarify this separation of concerns
• Code Implementation Issues. Clean up your code to ensure the following:
  • Methods should return or take as parameters abstractions such as List, Set, Map, Collection, or even Iterator rather than concrete types such as ArrayList or HashMap
  • Code should throw an exception rather than return null or simply call printStackTrace()

Not every team will have all of the issues above, but every team has at least one of the issues above. They represent a minimal set of design goals for this project that every team should be able to reach and, once you have verified that these issues are taken care of, you can use this time to clean up any aspect of the project's design.

Refactoring

Now that your team has considered how to improve the project's design, choose small refactorings to move forward implementing actual changes to your code to move it towards that design in a controlled manner. Let IntelliJ help you as much as possible with its Refactor menu and keep on top of any syntax errors that are produced with each change so it remains manageable and does not spiral out of control. Commit your changes to this lab's separate GIT branch frequently in case you need to roll back something.

Once you have verified that the primary issues and the formatting issues are taken care of, you can use the rest of the time to clean up any other aspect of the project's design or to add more tests.