if (checkTimeExpired(order.getCreated(), min)) { if (mailReports != null && mailReports.contains(order.getSessionId())) { return false; } else { return true; } } else { return false; }It does something simple but looks ugly and obscure. Half of the code actually deals with converting boolean values to boolean which is unnecessary. A lot of improvement can be made just by removing that:
return checkTimeExpired(order.getCreated(), min) && (mailReports == null || !mailReports.contains(order.getSessionId())This anti-pattern is suprisingly common, it is difficult to tell why. In this specific case it may have been used to break down a more complex logical statement to multiple simple ones at the price of a more complicated program structure. However the same goal can be achieved without drawbacks by refactoring, like Method Extraction:
return checkTimeExpired(order.getCreated(), min) && hasSession(mailReports, order.getSessionId());And possibly Move Method (if suitable):
return checkTimeExpired(order.getCreated(), min) && mailReports.hasSession(order.getSessionId())
These are not heavy changes by any means, but the difference in the quality of the resulting code is significant.
No comments:
Post a Comment