-
Notifications
You must be signed in to change notification settings - Fork 0
Refactoring
Here is the feedback from running JDeodorant on our source tree.
Refactoring Type Source Entity Target Class Entity Placement Rate it! Move Method cs310w10.MoleFinder.View.MoleMapListAdapter::makeMap(cs310w10.MoleFinder.Model.Mole):java.util.HashMap<java.lang.String,java.lang.String> cs310w10.MoleFinder.Model.Mole 0.882870511560153 Move Method cs310w10.MoleFinder.Model.MolesDataSource::insertPicture(cs310w10.MoleFinder.Model.Picture):void cs310w10.MoleFinder.Model.Picture 0.886298934745576 Move Method cs310w10.MoleFinder.Model.MoleFactoryTest::testDeleteMole():void cs310w10.MoleFinder.Model.Mole 0.8865316994726277 Move Method cs310w10.MoleFinder.Controller.PictureController::getUriAsString():java.lang.String cs310w10.MoleFinder.Model.Picture 0.8866867503640589 Move Method cs310w10.MoleFinder.Controller.PictureController::getDateAsString():java.lang.String cs310w10.MoleFinder.Model.Picture 0.886691179951399 Move Method cs310w10.MoleFinder.Controller.MoleController::associateMoleWithPicture(int):void cs310w10.MoleFinder.Model.Mole 0.8867691731444568 Move Method cs310w10.MoleFinder.Controller.MoleController::deleteMole():void cs310w10.MoleFinder.Model.Mole 0.8867698893153235 Move Method cs310w10.MoleFinder.Controller.PictureController::AssociatePictureWithMole(int):void cs310w10.MoleFinder.Model.Picture 0.8868117883967789 Move Method cs310w10.MoleFinder.Controller.PictureController::deletePicture():void cs310w10.MoleFinder.Model.Picture 0.886816014088077 Move Method cs310w10.MoleFinder.Controller.MoleControllerTest::testDeleteMole():void cs310w10.MoleFinder.Controller.MoleController 0.8870728694927894 Move Method cs310w10.MoleFinder.Model.MolesDataSource::deletePicture(cs310w10.MoleFinder.Model.Picture):void cs310w10.MoleFinder.Model.Picture 0.8870936033215319 Move Method cs310w10.MoleFinder.Model.PictureFactory::deletePicture(cs310w10.MoleFinder.Model.Picture):void cs310w10.MoleFinder.Model.Picture 0.8871087592882463 Move Method cs310w10.MoleFinder.View.ViewActivity::putPicture(android.content.Intent, cs310w10.MoleFinder.Model.Picture):void cs310w10.MoleFinder.Model.Picture 0.887113891834689 Move Method cs310w10.MoleFinder.View.ViewActivity::putMole(android.content.Intent, cs310w10.MoleFinder.Model.Mole):void cs310w10.MoleFinder.Model.Mole 0.887175462616641 Move Method cs310w10.MoleFinder.View.ViewActivity::launchEditMole():void cs310w10.MoleFinder.Model.Mole 0.887175462616641 Move Method cs310w10.MoleFinder.Controller.MoleControllerTest::testAssociateMoleWithPicture():void cs310w10.MoleFinder.Controller.MoleController 0.8874039160663103 Move Method cs310w10.MoleFinder.Model.MolesDataSource::deleteMole(cs310w10.MoleFinder.Model.Mole):void cs310w10.MoleFinder.Model.Mole 0.8874845550233893 Move Method cs310w10.MoleFinder.Model.MoleFactory::deleteMole(cs310w10.MoleFinder.Model.Mole):void cs310w10.MoleFinder.Model.Mole 0.8877770530356088 Move Method cs310w10.MoleFinder.Controller.MoleControllerTest::testEditMole():void cs310w10.MoleFinder.Controller.MoleController 0.8878464118751743 Move Method cs310w10.MoleFinder.Controller.MoleControllerTest::testCreateMole():void cs310w10.MoleFinder.Controller.MoleController 0.8882892301997855 current system 0.8884379254806629
None! Woot! I was really careful about this to begin with... ~Claire
cs310w10.MoleFinder.Controller.ListMoleController public int addMole(java.lang.String, java.lang.String, java.lang.String) newmole B1 0/4 cs310w10.MoleFinder.Controller.MoleControllerTest public void testEditMole() mole B1 0/4 cs310w10.MoleFinder.Controller.PictureController private android.net.Uri makeFile(android.graphics.Bitmap, java.lang.String) folder B1 0/3 cs310w10.MoleFinder.View.EditImageViewActivity public void pressSubmitButton() controller B1 0/3 cs310w10.MoleFinder.View.ListMoleViewActivity public void onItemClick(AdapterView<?>, android.view.View, int, long) moleId B1 0/3 cs310w10.MoleFinder.View.SearchViewActivity public void onCreate(android.os.Bundle) lController B1 0/3 cs310w10.MoleFinder.Controller.PictureController private android.net.Uri makeFile(android.graphics.Bitmap, java.lang.String) filename B1 0/2 cs310w10.MoleFinder.View.NewMoleViewActivity public void pressSubmitButton() mcontroller B1 5/9 cs310w10.MoleFinder.View.NewMoleViewActivity public void pressSubmitButton() mcontroller B2 1/5 cs310w10.MoleFinder.Model.MolesDataSource public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B1 6/11 cs310w10.MoleFinder.Model.MolesDataSource public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B2 4/9 cs310w10.MoleFinder.Model.MolesDataSource public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B3 3/8 cs310w10.MoleFinder.Model.PictureFactory public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B1 6/11 cs310w10.MoleFinder.Model.PictureFactory public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B2 4/9 cs310w10.MoleFinder.Model.PictureFactory public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B3 3/8 cs310w10.MoleFinder.Controller.ListPictureController public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B1 7/12 cs310w10.MoleFinder.Controller.ListPictureController public ArrayList<cs310w10.MoleFinder.Model.Picture> getListPictureFromMole(int) picture B3 3/8
cs310w10.MoleFinder.Model.PictureTrue [cs310w10.MoleFinder.Model.PictureTrue::java.util.ArrayList<java.lang.Integer> moleID, cs310w10.MoleFinder.Model.PictureTrue::addMoleID(java.lang.Integer):void] 0.9205184429308406 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::getAllPictures():java.util.ArrayList<cs310w10.MoleFinder.Model.Picture>, cs310w10.MoleFinder.Model.MolesDataSource::cursorToPicture(android.database.Cursor):cs310w10.MoleFinder.Model.Picture] 0.9220376797157843 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::createMole(java.lang.String, java.lang.String, java.lang.String):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Model.MolesDataSource::getAllMoles():java.util.ArrayList<cs310w10.MoleFinder.Model.Mole>, cs310w10.MoleFinder.Model.MolesDataSource::getMoleFromId(long):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Model.MolesDataSource::editMole(cs310w10.MoleFinder.Model.Mole, java.lang.String, java.lang.String, java.lang.String):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Model.MolesDataSource::deleteMole(cs310w10.MoleFinder.Model.Mole):void, cs310w10.MoleFinder.Model.MolesDataSource::deleteAllMoles():void, cs310w10.MoleFinder.Model.MolesDataSource::cursorToMole(android.database.Cursor):cs310w10.MoleFinder.Model.Mole] 0.9235241152830481 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::cs310w10.MoleFinder.Model.MoleSQLiteHelper moledbhelper, cs310w10.MoleFinder.Model.MolesDataSource::close():void, cs310w10.MoleFinder.Model.MolesDataSource::open():void, cs310w10.MoleFinder.Model.MolesDataSource::insertMolePicturePair(int, int):void, cs310w10.MoleFinder.Model.MolesDataSource::getPhotoIdsFromeMole(int):java.util.ArrayList<java.lang.Integer>] 0.9246347425054258 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::cs310w10.MoleFinder.Model.MoleSQLiteHelper moledbhelper, cs310w10.MoleFinder.Model.MolesDataSource::close():void, cs310w10.MoleFinder.Model.MolesDataSource::open():void, cs310w10.MoleFinder.Model.MolesDataSource::insertMolePicturePair(int, int):void, cs310w10.MoleFinder.Model.MolesDataSource::getPhotoIdsFromeMole(int):java.util.ArrayList<java.lang.Integer>, cs310w10.MoleFinder.Model.MolesDataSource::createMole(java.lang.String, java.lang.String, java.lang.String):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Model.MolesDataSource::getAllMoles():java.util.ArrayList<cs310w10.MoleFinder.Model.Mole>, cs310w10.MoleFinder.Model.MolesDataSource::getMoleFromId(long):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Model.MolesDataSource::editMole(cs310w10.MoleFinder.Model.Mole, java.lang.String, java.lang.String, java.lang.String):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Model.MolesDataSource::deleteMole(cs310w10.MoleFinder.Model.Mole):void, cs310w10.MoleFinder.Model.MolesDataSource::deleteAllMoles():void, cs310w10.MoleFinder.Model.MolesDataSource::cursorToMole(android.database.Cursor):cs310w10.MoleFinder.Model.Mole] 0.9246613228161775 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::cs310w10.MoleFinder.Model.MoleSQLiteHelper moledbhelper, cs310w10.MoleFinder.Model.MolesDataSource::close():void, cs310w10.MoleFinder.Model.MolesDataSource::open():void] 0.9246924395979937 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::insertPicture(cs310w10.MoleFinder.Model.Picture):void, cs310w10.MoleFinder.Model.MolesDataSource::getListPictureFromMole(int):java.util.ArrayList<cs310w10.MoleFinder.Model.Picture>, cs310w10.MoleFinder.Model.MolesDataSource::deletePicture(cs310w10.MoleFinder.Model.Picture):void] 0.9250011488425807 cs310w10.MoleFinder.Model.MolesDataSource [cs310w10.MoleFinder.Model.MolesDataSource::insertMolePicturePair(int, int):void, cs310w10.MoleFinder.Model.MolesDataSource::getPhotoIdsFromeMole(int):java.util.ArrayList<java.lang.Integer>] 0.9251227395426305 cs310w10.MoleFinder.Controller.MoleController [cs310w10.MoleFinder.Controller.MoleController::deleteMole():void, cs310w10.MoleFinder.Controller.MoleController::associateMoleWithPicture(int):void] 0.9244514596536982 cs310w10.MoleFinder.Controller.MoleController [cs310w10.MoleFinder.Controller.MoleController::createMole(java.lang.String, java.lang.String, java.lang.String):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Controller.MoleController::getMoleFromId(int):cs310w10.MoleFinder.Model.Mole, cs310w10.MoleFinder.Controller.MoleController::editMole(java.lang.String, java.lang.String, java.lang.String):void, cs310w10.MoleFinder.Controller.MoleController::deleteMole():void, cs310w10.MoleFinder.Controller.MoleController::associateMoleWithPicture(int):void, cs310w10.MoleFinder.Controller.MoleController::cursorToMole(android.database.Cursor):cs310w10.MoleFinder.Model.Mole] 0.9248532435299862 cs310w10.MoleFinder.Controller.MoleController [cs310w10.MoleFinder.Controller.MoleController::cs310w10.MoleFinder.Model.Mole mole, cs310w10.MoleFinder.Controller.MoleController::cs310w10.MoleFinder.Model.MoleSQLiteHelper connection, cs310w10.MoleFinder.Controller.MoleController::getPhotoIdsFromeMole(int):java.util.ArrayList<java.lang.Integer>] 0.9248532435299862 cs310w10.MoleFinder.Controller.PictureController [cs310w10.MoleFinder.Controller.PictureController::getDateAsString():java.lang.String, cs310w10.MoleFinder.Controller.PictureController::getUriAsString():java.lang.String] 0.9246678786763086 cs310w10.MoleFinder.Controller.PictureController [cs310w10.MoleFinder.Controller.PictureController::deletePicture():void, cs310w10.MoleFinder.Controller.PictureController::AssociatePictureWithMole(int):void] 0.9246729066409162 cs310w10.MoleFinder.Controller.ListPictureController [cs310w10.MoleFinder.Controller.ListPictureController::getPictureById(int):cs310w10.MoleFinder.Model.Picture, cs310w10.MoleFinder.Controller.ListPictureController::deleteListPicture():void] 0.9247345817786862 cs310w10.MoleFinder.Controller.MoleControllerTest [cs310w10.MoleFinder.Controller.MoleControllerTest::testCreateMole():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testEditMole():void] 0.9251127460821408 cs310w10.MoleFinder.Controller.MoleControllerTest [cs310w10.MoleFinder.Controller.MoleControllerTest::testDeleteMole():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testGetMoleFromId():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testAssociateMoleWithPicture():void] 0.9251182391446688 cs310w10.MoleFinder.Controller.MoleControllerTest [cs310w10.MoleFinder.Controller.MoleControllerTest::testCreateMole():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testEditMole():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testDeleteMole():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testGetMoleFromId():void, cs310w10.MoleFinder.Controller.MoleControllerTest::testAssociateMoleWithPicture():void] 0.9251205804612452 cs310w10.MoleFinder.Model.MoleFactoryTest [cs310w10.MoleFinder.Model.MoleFactoryTest::testCreateMole():void, cs310w10.MoleFinder.Model.MoleFactoryTest::testEditMole():void, cs310w10.MoleFinder.Model.MoleFactoryTest::testDeleteMole():void] 0.9251217642751624 cs310w10.MoleFinder.Model.MoleFactoryTest [cs310w10.MoleFinder.Model.MoleFactoryTest::cs310w10.MoleFinder.Model.MolesDataSource factory, cs310w10.MoleFinder.Model.MoleFactoryTest::cs310w10.MoleFinder.Model.Mole testmole, cs310w10.MoleFinder.Model.MoleFactoryTest::testMoleFactory():void, cs310w10.MoleFinder.Model.MoleFactoryTest::testCreateMole():void, cs310w10.MoleFinder.Model.MoleFactoryTest::testEditMole():void, cs310w10.MoleFinder.Model.MoleFactoryTest::testDeleteMole():void] 0.9251239637608789 cs310w10.MoleFinder.Model.MoleFactoryTest [cs310w10.MoleFinder.Model.MoleFactoryTest::cs310w10.MoleFinder.Model.MolesDataSource factory, cs310w10.MoleFinder.Model.MoleFactoryTest::cs310w10.MoleFinder.Model.Mole testmole, cs310w10.MoleFinder.Model.MoleFactoryTest::testMoleFactory():void] 0.9251277478535777 cs310w10.MoleFinder.View.ViewActivity [cs310w10.MoleFinder.View.ViewActivity::putMole(android.content.Intent, cs310w10.MoleFinder.Model.Mole):void, cs310w10.MoleFinder.View.ViewActivity::launchEditMole():void] 0.9251225082306559 cs310w10.MoleFinder.Model.MolesDataSourceTest [cs310w10.MoleFinder.Model.MolesDataSourceTest::cs310w10.MoleFinder.Model.MolesDataSource source, cs310w10.MoleFinder.Model.MolesDataSourceTest::cs310w10.MoleFinder.Model.Mole testmole, cs310w10.MoleFinder.Model.MolesDataSourceTest::testMolesDataSource():void] 0.9251249573460077
From the results of the JDeodorant, these are our chosen refactorings and justifications for each.
Nothing to do.
move makeMap from MoleMapListAdapter to Mole - done
move database functions to model classes - no, we're trying to keep the database separate from the model objects. We've had some discussion about this.
move test functions - no, they should stay in the test classes. Apparently JDeodorant isn't designed to work with JUnit.
move fancier getUri and getDate methods - done.
move some ViewActivity functions into the views that use them - we don't want to do this because it limits the possibility of adding new views which use those functions. They're not hurting anything where they are.
That pretty much covers all of them. Most of them are about moving database functions into model classes or moving test functions around.
ListMoleController is deprecated anyway so we're ignoring those suggestions.
Again, some suggestions to fiddle with the junit tests..
Extract the folder method from makeFile? Okay, but I'm gonna move the mkdirs in there as well. done.
Extract the picture updating from the submitbutton call... done.
Extract getMoleFromList from ListMoleView's onItemClick... done.
Extract filename from makeFile? sorry, it's two lines of code with zero chance of being reused elsewhere. no thanks.
NewMoleViewActivity? I forgot this even existed, and it's still on our old architecture. EditMole more than does the job as it is.
Extract the cursor use from the various MolesDataSource areas? They are pretty consistent as it is so it'd remove clarity to change it.
PictureFactory: also never used.
PictureTrue - deprecated
MolesDataSource: This is probably our strongest candidate for splitting up into smaller components, but it's hard to do that and still be able to edit the SQL calls with consistency.
MoleController: We're not going to extract the 'mole' concept from it because we already have a 'mole' concept.
PutMole and other communicative View functions - these should be kept close together for consistent behaviour.
Unit tests: See above.