-
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)
- [CacheIO] (#wiki-cacheIO)
- [CommentController] (#wiki-commentControl)
- [CommentListModel] (#wiki-commentListModel)
- [CommentManager] (#wiki-commentManager)
- [CommentModel] (#wiki-commentModel)
- [CommentPush] (#wiki-commentPush)
- [CommentRetriever] (#wiki-commentRetriever)
- [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 - file - IGNORED
Before: JDeodorant recognized that there was some code that could be extracted out of the CacheIO class.
Recommended Action: to extract the file IO out of the CacheIO class.
Action: Ignored as it file IO is intended responsibility of this class.
Result: CacheIO is still responsible for the Cache file IO.
- God Class - path + load - IGNORED
Before: JDeodorant recognized that there was some code that could be extracted out of the CacheIO class.
Recommended Action: to extract the file IO out of the CacheIO class.
Action: Ignored as it file input is intended responsibility of this class.
Result: CacheIO is still responsible for the Cache file input.
--
- 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 - retrieve- FIXED
Before: JDeodorant identified extraction of some of the responsibilities of the CommentManager class.
Recommended Action: The JDeodorant recommended action is to extract the functionality required to retrieve comments from local storage.
Action: Created new class CommentRetriever
Result: CommentRetriever will contain functionality required to retrieve comments from Bookmarks, myComments and favorites.
- God Class - refresh - IGNORED
Before: JDeodorant identified extraction of some of the responsibilities of the CommentManager class.
Recommended Action: The JDeodorant recommended action is to extract the functionality required to refresh Bookmarks, myComments and favorites.
Action: Ignored as this will not reduce responsibilities of the class by a level significant enough to warrant changes.
Result: refreshMyComments, refreshMyBookmarks and refreshMyFavourites methods are still within the CommentManager class.
- God Class - new + comment - IGNORED
Before: JDeodorant identified extraction of some of the responsibilities of the CommentManager class.
Recommended Action: The JDeodorant recommended action is to extract the functionality required to update the cache.
Action: Ignored as this is seemingly an intended responsibility of the class.
Result: CommentManager is still responsible for updating the caching system with new comments.
- Feature Envy - refresh - IGNORED
Before: JDeodorant determined that the refresh method was unwarranted within the CommentManager class.
Recommended Action: The JDeodorant recommended action is to move the method to the Cache class.
Action: Ignored as this is not a reasonable responsibility of the Cache class.
Result: CommentManager is still responsible for loading comments into the CommentListModel.
- Feature Envy - newTopLevel - IGNORED
Before: JDeodorant determined that the newTopLevel method was unwarranted within the CommentManager class.
Recommended Action: The JDeodorant recommended action is to move the method to the GeoTopicsApplication class.
Action: Ignored as this is not a reasonable responsibility of the GeoTopicsApplication class.
Result: CommentManager is still responsible for putting newly created TopLevel comments in the cache, and calling the push method if network is available.
- Feature Envy - newReply - IGNORED
Before: JDeodorant determined that the newReply method was unwarranted within the CommentManager class.
Recommended Action: The JDeodorant recommended action is to move the method to the GeoTopicsApplication class or CommentModel class.
Action: Ignored as this is not a reasonable responsibility of the GeoTopicsApplication or CommentModel class.
Result: CommentManager is still responsible for putting newly created ReplyLevel comments into myComments.
- Feature Envy - updateComment - IGNORED
Before: JDeodorant determined that the updateComment method was unwarranted within the CommentManager class.
Recommended Action: The JDeodorant recommended action is to move the method to the GeoTopicsApplication class.
Action: Ignored as this is not a reasonable responsibility of the GeoTopicsApplication class.
Result: CommentManager is still responsible for either pushing the Comment online or stashing it locally until network is available.
- Feature Envy - pushStashedComments - IGNORED
Before: JDeodorant determined that the pushStashedComments method was unwarranted within the CommentManager class.
Recommended Action: The JDeodorant recommended action is to move the method to the GeoTopicsApplication class.
Action: Ignored as this is not a reasonable responsibility of the GeoTopicsApplication class.
Result: CommentManager is still responsible for pushing stashed comments.
- 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
- Long Method smells - IGNORED
Before: JDeodorant identified a couple long methods (getMyFavourites and getMyBookmarks)in the CommentRetriever class.
Recommended Action: JDeodorant recommended extracting 2 lines out of 5 line methods
Action: Ignored as these methods were not long in the first place
Result: code stayed the same.
- Feature Envy smell - getCommentByComboID - IGNORED
Before: JDeodorant identified a method (getCommentByComboID) that was determined to belong in the User class.
Recommended Action: JDeodorant recommended moving this method to the User class.
Action: Ignored as this functionality is out of place for the User class.
Result: method remains in the CommentRetriever class.
- 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.