-
Notifications
You must be signed in to change notification settings - Fork 3
Refactoring
#Step-by-step Re-factoring Problems & Solutions
- Please ensure to run God Class, Long Method, Type Checking and Feature Envy while using JDeodorant.
- [AModel] (#wiki-aModel)
- [AView] (#wiki-aView)
- [BitmapJsonConverter] (#wiki-bitmapJson)
- [BrowseActivity] (#wiki-browse)
- [BrowseView] (#wiki-browseView)
- [Cache] (#wiki-cache)
- [CommentController] (#wiki-commentControl)
- [CommentListModel] (#wiki-commentListModel)
- [CommentManager] (#wiki-commentManager)
- [CommentModel] (#wiki-commentModel)
- [CommentPush] (#wiki-commentPush)
- [CommentPull] (#wiki-commentPull)
- [CommentSearch] (#wiki-commentSearch)
- [CreateCommentActivity] (#wiki-createComment)
- [EditCommentActivity] (#wiki-editComment)
- [EditMyProfileActivity] (#wiki-editMyProfile)
- [ElasticSearchResponse] (#wiki-elasticSearchResponse)
- [ElasticSearchSearchResponse] (#wiki-elasticSearchSearch)
- [GeoTopicsApplciation] (#wiki-geoTopicsApplication)
- [Hits] (#wiki-hits)
- InspectCommentActivity
- InspectOtherProfilesActivity
- InspectProfileActivity
- ManualLocationActivity
- [MapsActivity] (#wiki-maps)
- [MyBookmarksActivity] (#wiki-myBookmarks)
- [MyCommentsActivity] (#wiki-myComments)
- [MyFavouritesActivity] (#wiki-myFavourites)
- [ProfilePush] (#wiki-profilePush)
- [ProfileSearch] (#wiki-profileSearch)
- [CacheIO] (#wiki-CacheIO)
- [ReplyLevelActivity] (#wiki-replyLevel)
- [StartActivity] (#wiki-start)
- [TopLevelActivity] (#wiki-topLevel)
- [User] (#wiki-user)
- [UserController] (#wiki-userController)
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- God Class - File + Load - FIXED
Before: JDeodorant recognized that the cache class had too many responsibilities and such it was too large.
Recommended Action: Recommended that we encapsulate the cache IO into it's own class.
Action: Moved the cache IO code using JDeodorant then renamed the class CacheIO to be more in line with its actual duties.
Result: The cache class now handles the internal working of the cache however it does not handle read and write to disk. This is delegated to the CacheIO class now.
- God Class - cache - FIXED
Before: JDeodorant recognized that the cache class had more method's it wanted to extract due to too many responsabilities
Recommended Action: The JDeodorant recommended extractions left the cache class with zero responsibilities so we recommend manually doing a few of the suggested changes but not the rest. Ignoring the rest of the JDeodorantgod class recommendations.
Action: Moved the path variable into the CacheIO class and initialize it through the constructor. Also moved the load method into the CacheIO since it involves direct cache IO.
Result: The cache class now has a smaller amount of responsibilities making it a more concise class.
- Feature Envy - Move Method - FIXED
The above re factoring removed the JDeodorant recommendations for feature envy.
- God Class - update + manage + comment - IGNORED
Before: JDeodorant recognized that there was some code that could be extracted out of the CommentController class.
Recommended Action: The JDeodorant recommended action was to put the commentManager reference and the updateComment code into another class file.
Action: Ignoring this suggestion as we feel the act of modifying a comment should stay inside the controller. Re factoring this will also mean the commentController does nothing but delegate responsibility and would have no other responsibilities.
Result: The CommentController class will not change, it will still maintain the responsibility of updating editable comments.
- Feature Envy - updateComment - IGNORED
Before: JDeodorant recognized that the updateComment method was both modifying the contents of the comment and handing off the comment to the comment manager to be put onto the internet.
Recommended Action: The JDeodorant recommended action was to move the modification of the comment into the manager.
Action: Feel that this is uncessesary as it will likely just promp JDeodorant to want to move it out of that class as well. Feel that the CommentController is the best MVC location to be directly editing the model.
Result: Nothing has changed the code will remain where it is.
- Feature Envy - newReply - FIXED
Before: JDeodorant recognized that the controller was both delegating to the comment manager and the user class in order to make a new reply level comment.
Recommended Action: The JDeodorant recommended action was to extract the method into its own class and call it from CommentController.
Action: Opted for a simpler solution. Moved the saving of a new comment into the user's list of my comments into the comment manager.
Result: The CommentController no longer has to delegate to 2 different class' in order to create a new comment. It delegates to the comment manager who passes it along to the user after it is done pushing it to the internet.
- Feature Envy - newTopLevel - FIXED
Before: JDeodorant recognized that the controller was both delegating to the comment manager and the user class in order to make a new top level comment.
Recommended Action: The JDeodorant recommended action was to extract the method into its own class and call it from CommentController.
Action: Opted for a simpler solution. Moved the saving of a new comment into the user's list of my comments into the comment manager.
Result: The CommentController no longer has to delegate to 2 different class' in order to create a new comment. It delegates to the comment manager who passes it along to the user after it is done pushing it to the internet.
- Long Method - setList - IGNORED
Before: JDeodorant identified a long method in CommentListManager setList.
Recommended Action: JDeodorant recommented extracting the first half of the method into its own method.
Action: Ignored this as the original method is only 4 lines long and such making two 2 line method's does not add to the readability of the code or its efficency.
Result: The class is unchanged from the beginning. It still has the 4 line setList method.
- God Class - sort - FIXED
Before: JDeodorant recognized that the CommentListModel had too many responsibilities.
Recommended Action: The JDeodorant recommended action was to extract the sort code from the CommentListModel.
Action: The JDeodorant extraction was a bit messy and contained some unnecessary extractions. Manually extracted the sort code from CommentListModel and put it in its own class.
Result: The CommentListModel now register's its list with a comment sorter and delegates the sorting of this list to that instance of the class.
- God Class - new + M - NOT FIXED
- God Class - comment + refresh - NOT FIXED
- Feature Envy - refresh - NOT FIXED
- Feature Envy - refreshMyComments - NOT FIXED
- Feature Envy - refreshMyBookmarks - NOT FIXED
- Feature Envy - refreshMyFavourites - NOT FIXED
- Feature Envy - newTopLevel - NOT FIXED
- Feature Envy - newReply - NOT FIXED
- Feature Envy - updateComment - NOT FIXED
- Feature Envy - getComment - NOT FIXED
- God Class - geoloc - IGNORED
Before: JDeodorant identified a god class with too many responsibilities in the CommentModel class.
Recommended Action: The JDeodorant recommended action is to extract the geoLocation code out into a new class.
Action: Ignoring this as the geoLocation code for the CommentModel is only a set of getter and setters for the comments geo location. Extracting this seems excessive as there is no logic involved with this code we would simply be delegating the storage and retrieving of a comments location to another class.
Result: The commentModel will maintain the getter and setters for its geo location.
- God Class- level + M + es + top - IGNORED
Before: JDeodorant identified extraction of some of the responsibilities of the CommentModel class.
Recommended Action: The JDeodorant recommended action was to extract the code involved with the main fields of the commentModel such as, title, author, picture, etc.
Action: Ignoring this as the CommentModel class is simply storage for all the data necessary for a comment. It has no logic beyond constructors and a long list of getters and setters for each of its fields. Most of these are 1 line method's so extracting any of it would save us nothing as we would still need the code to delegate to the other class.
Result: The commentModel class while it is long is a simple data storage class with no real logic involved so we opted to leave it the same.
- No Bad Smells
- No Bad Smells
- God Class - model + pull + comment - FIXED Before: JDeodorant identified a god class with too many responsibilities in the CommentModel class.
Recommended Action: JDeodorant recommended extracting the pullComment and pull methods into a separate class.
Action: Extracted the pullComment and pull methods into the newly created class: CommentPull
Result: CommentSearch will now call the CommentPull.pullComment and CommentPull.pull methods to pull comments off the ES server.
- God Class - repli - NOT FIXED
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- God Class - context + network + available- IGNORED
Before: JDeodorant identified too many responsibilities in the geo topics application singleton.
Recommended Action: The JDeodorant recommended action was to extract the context and the isNetworkAvailable code into another class.
Action: Ignored this as it made sense for the application singleton to be the one that the rest of the application could ask for the network availability status. Also left the context in the singleton as this is a reference to a general context used for things like reading and writing to disk and other tasks that require a context outside an activity but do not need a specific one.
Result: The geo topics application singleton will stay the same.
- No Bad Smells
- No Bad Smells
# InspectOtherProfilesActivity
- Feature Envy - updateUi - IGNORED
Before: JDeodorant identified a feature envy in the code that updated the views related to viewing other users profiles.
Recommended Action: JDeodorant recommended moving the updateUI method from the InspectOtherProfilesActivity into the User class.
Action: Ignored this suggestion. The updating of the activities views is best left in the activity and not in the user. The recommended move would require passing references to the view into the user class which seems redundant.
Result: The InspectOtherProfilesActivity remains the same as it was before. The activity will update its own views.
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- No Bad Smells
- God Class - location - FIXED
Before: JDeodorant recognized that the user class had too many responsibilities and was much too large.
Recommended Action: The JDeodorant recommended extractions were to move the location related code into another class.
Action: Moved the location related code into another class to encapsulate it as UserLocationServices. Also moved any supporting code that was only used by these functions. Had to manually clean up the re factored class as it contained unnecessary code that was never used as well as some poorly named elements.
Result: The user class now delegates its location based functionality to a helper class.
- God Class - file - FIXED
Before: JDeodorant recognized that the cache class handled IO when it could delegate it to another helper class.
Recommended Action: The JDeodorant recommended the extraction of some of the IO in the user class.
Action: Moved the recommented IO plus moved the rest that it did not suggest to move. Re worked the main load/store code for the user in order to allow it to move to another file.
Result: The user class no longer directly handles and of it's disk IO operations it delegates them to the UserIO class. The main user class is now much shorter and has less responsibilities.
- God Class - ID, Favourites, Bookmarks, comments & Profile - IGNORED
Before: JDeodorant determined that the code for handling the users profile ID, list of favourites, list of bookmarks and other profile code should be moved to individual class'
Recommended Action: Recommend not implementing any of these changes. This would involve creating many class files which would make the user class much harder to manage. On top of this the responsibilities of the user class would reduce to meer delegation and we feel that the user class is completly capable of keeping track of its own list of favourites, bookmarks and comments as well as the few fields in its profile.
Action: Did not take any.
Result: The user class remains in controls of its direct fields such as user name, profile picture and its list of favourites.
- Feature Envy - favourite - FIXED
Before: JDeodorant identified feature envy in the favourite method.
Recommended Action: The JDeodorant recommendation was to move this method to the user class.
Action: Moved the method that handled adding or removing favourites into the user class.
Result: The user controller no longer decides if it removes or adds a favourite it simply delegates to the user class which is the rightful place where this decision should be made.
- Feature Envy - bookmark - FIXED
Before: JDeodorant identified feature envy in the bookmark method. This method is handling responsibilities that are not for the controller to handle.
Recommended Action: The JDeodorant recommendation was to move this method to the user class.
Action: Moved the method that handled adding or removing bookmarks into the user class.
Result: The user controller no longer decides if it removes or adds a bookmark it simply delegates to the user class which is the rightful place where this decision should be made.