If you find any nested statements in your code, make sure you simplify them before any further changes. As a rule do not nest more than 3 if/for/while/switch/try statements.
IssueTracker:
Nested statements
If there are more than three nested statements then the code starts looking really messy. The more complex a code is, the more painful the refactoring becomes. In the below class, we show multiple nested statements, the statementif (relatedIssue.isResolved())
is the fifth nested statement. Since the code is tangled, the anti-pattern is called "Spaghetti code".IssueTracker:
package refactor; import java.util.List; /** * Nested loops */ public class IssueTracker { public void resolve(Issue issue) { if (issue.isAssigned()) { issue.resolve(); List relatedIssues = issue.getRelatedIssues(); boolean closeIssue = true; if (relatedIssues != null) { for (Issue relatedIssue : relatedIssues) { try { if (!relatedIssue.isResolved()) { closeIssue = false; break; } } catch (Exception e) { //log exception } } } if (closeIssue) { issue.close(); } } } }
Nested statements simplified
We simplify the above code by first making sure all the pre-conditions are met.- The issue must is in 'Assigned' status, if not, we don't have to do anything further.
- Similarly, we also want to close the issue if the related issues are all resolved.
- Write a separate method with an explicit name that indicating our intention.
- We want to know whether all our related issues are resolved, we merge the below into a new method
verifyRelatedIssuesResolved()
List relatedIssues = issue.getRelatedIssues(); boolean closeIssue = true; if (relatedIssues != null) { for (Issue relatedIssue : relatedIssues) { try { if (!relatedIssue.isResolved()) {
- Once it is confirmed all are resolved, close the issue.
package refactor.clean; import refactor.Issue; import java.util.List; /** * Nested loops simplified */ public class IssueTracker { public void resolve(Issue issue) { if (!issue.isAssigned()) { return; } issue.resolve(); boolean closeIssue = verifyRelatedIssuesResolved(issue); if (closeIssue) { issue.close(); } } private boolean verifyRelatedIssuesResolved(Issue issue) { List relatedIssues = issue.getRelatedIssues(); if (relatedIssues == null || relatedIssues.isEmpty()) { return true; } for (Issue relatedIssue : relatedIssues) { if (!relatedIssue.isResolved()) { return false; } } return true; } }
No comments:
Post a Comment