kishanvshah007/SOEN-6471-iFreeBudget-Milestone-4 — pull request #3 by kishanvshah007 (2014-04-08 00:35:24+00:00)
Title: Visitor pattern

Code Smells and System Level Refactorings
1 Multiple if statements, chain of “instanceof” expressions
Location: NotificationDisplayFactory Class in notifications Package
![untitled](https://cloud.githubusercontent.com/assets/2430888/2638768/63768ac4-beb5-11e3-8368-87108f7f097f.jpg)
Problem: Chains of if statements and all of whose conditions are “instanceof” expressions. Such constructions usually indicate a failure of object-oriented design, which dictates that such type-based dispatch should be done via polymorphic method calls rather than explicit chains of type tests.
Solution: Visitor pattern (https://en.wikipedia.org/wiki/Visitor_pattern)
Originally, the visitor design pattern is a way of separating an algorithm from an object structure on which it operates. (Wikipedia) However, this pattern suits our case due to its major characteristic: “double dispatch”. In our case, statement “notification instanceof XXXNotification” is the first dispatch decided at run time. According to the TYPE of notification object, the program has different “return new XXXNotificationFrame()”, that can be considered the second dispatch. After refactoring, if we need to ADD a new type of “notification” or new behaviors, we won’t break the present code.
Mechanics: The visitor pattern looks complicated, but since the actual class structure highly match this pattern and due to writing space limitation, we will briefly go through it.
(1) rename interface “UINotification” to “INotificationElement”, and ADD a new empty method “accept(INotificationElementVisitor visitor)” in this interface
(2) Since Class AlertNotification, UpdateCheckNotification…already implement “UINotification”, we just need to implement the NEW “accept(INotificationElementVisitor visitor) { visitor.visit(this);}” method in each derived class.
(3) Create a new interface called “INotificationElementVisitor” with following empty method: visit(AlertNotification alertNotification); visit(UpdateCheckNotification updateCheckNotification)…
(4) Create a new class called “NotificationElementVisitor” and implement the new interface “INotificationElementVisitor”, different visit(XXXNotification XXX) with different “return new XXXNotificationFrame()”
Finally, the code snippets we saw, all if statement will gone:
notification.accept (new NotificationElementVisitor (frame));
2 Extend Adapter instead of implementing Listener
Location: User defined Class -- Class AccountsTreeDragSource, AccountsTreeDropTarget
![image](https://cloud.githubusercontent.com/assets/2430888/2638771/7c7fc2b0-beb5-11e3-9f16-dc3a950daf1b.png)

Problem: Classes (Not autogenerated GUI class) which implement a listener, often better extend the corresponding adapter instead. Listener is an Interface, Interface force we implement every method it defined regardless whether we need the method or not. The result is many redundant empty methods left which make the code less clean and difficult to read.
Solution: Extend Adapter instead of implementing Listener
Adapter, usually do nothing by default, is just an empty implementation of Listener. With Adapter, we only need to override those methods that we are interest to deal with. In GRASP term, we are provide a layer of Indirection which reduces redundancy and increases readability.
Mechanics:
(1) Change Class declaration, “class AccountsTreeDragSource implements DragSourceListener {…}” to “class AccountsTreeDragSource extends DragSourceAdapter {…}”
(2) Delete all those empty non-used method, which is the code snippets above in the original Class.

![untitled](https://cloud.githubusercontent.com/assets/2430888/2638781/a7c610fa-beb5-11e3-85e6-1f1db5d9c9cd.png)