• Commits Page: https://github.com/Ottomoeller/ScavengrRefactoring/commits/master

List of Refactorings

Refactoring 0: Testing

Issue Being Addressed:

There are both Geb and Unit tests, but we need to produce more coverage, especially on controllers.

Planned Refactoring:

Go through the current tests to see what has already been covered. Then make more tests for classes that have little or no coverage. It looks like none of the controller tests are actually doing anything. There are also a few geb tests that may or may not be working.

Our plan will be to divide into two groups: one group working on Geb tests, while the other works on Unit tests.

Goals for Geb Testing:

• Double check that there is at least one test per page.
• Uncomment commented out code and see if each of these tests work.
• Some Geb tests are empty. For each test class that is empty or made add at least one basic test that isn't the user login.

Goals for Unit Testing:

• Check to see which controllers have tests. If there are missing tests make them.(We can use the existing tests as examples)
• Analyze what each of the existing tests do and if a Geb test can do the same thing make a Geb test instead.

Technical Difficulty:

The difficulty will be medium because of an un-refactored code base as well as a large amount of code to test. Also geb and controller tests have proven to be tricky in the past.

Impact on the Code:

This will first tell us if any of the code is broken. It will also help let us know if any of our refactoring did something it shouldn't, making it harder for us to break the project.

Refactoring 1: Duplicate code

Issue Being Addressed:

There are multiple cases where code is the same, some duplicates are in the same class or go into multiple classes. Such an example of this are lines 492 and 508 of the huntcontroller. Also lines 236, 241, 253, and 257 of huntcontroller (though we dont know if these fixes will actually work).

Planned Refactoring:

Go through and find other examples of duplicate like the few we specified in hunt controller. **Update: We ran a Simian report on the project and now are addressing the duplications from there.

Technical Difficulty:

This will be fairly difficult due to the unknowns about the fixes actually working.

Impact on the Code:

This will just make the code base easier to read and remove duplicate logic.

Refactoring 2: Reducing method sizes in domain classes

Issue Being Addressed:

There are cases where a method can be reduced either through helper methods or combining if/for statements. one such example is in the domain class prompt where at lines 29 and 32 can be merged into one single if statement.

Planned Refactoring:

So we have multiple examples that we can refactor, all of which deal with the equals method in each of the domain classes.

Technical Difficulty:

This will not be difficult at all because it is simply just deleting the one if statement and putting its condition as an or statement in the other if case.

Impact on the Code:

This will shorten the code base and reduce redundancy.

Refactoring 3: Renaming variables

Issue Being Addressed:

There are variable names that are not very descriptive as to their purpose. One such example is in controller class huntcontroller at line 64 where the ByteArrayOutsputStream's name is baos, which stands for its data type.

Planned Refactoring:

Go through and change names that are not very descriptive as to their purpose. Like baos in huntcontroller line 64 and the variable e in huntcontroller on line 103.

Technical Difficulty:

This wont be all to difficult the only issue with this is going through the entire code and determining whether a name is descriptive enough to get its meaning understood.

Impact on the Code:

This will make the code base easier to read.

Refactoring 4: Refactor Large controller classes

Issue Being Addressed:

A number of the controller classes are very large, being several hundreds of lines long. In addition many of these classes have rather large methods, such as in the huntController class were the create() is nearly 90 lines long.

Planned Refactoring:

probably want to go through the huntController,promptController and photoController classes and try to break down some of the methods. Alternatively it may make more sense to actually extract some of the methods into their own class such as downloadPhotos() in huntController.

Technical Difficulty:

This is probably going to be about medium difficulty. Since the controllers are were most of the front end functionality lives. As such they touch a lot of things and given the length of many of the methods it will probably take a while to get a good grasp on how everything works.

Impact on the Code:

Reducing the size of the controller classes will make the controllers substantially more readable and also less intimidating to try and understand what's going on.

---

Outcomes

Refactoring 0 - Testing

After adding some basic unit/spock tests for the domain classes, we focused on Geb testing because they alone covered what we needed. We fixed and created tests for CreateAHuntPage, HomePage, HuntShowPage, ListOfHuntsPage, and UserShowPage. These tests go through the basic functions of each page. There is still a lot of pages and functionality to test such as prompts and editing for pages but we will, for the time being, test those manually.

Refactoring 1 - Duplicate Code

We were able to run Simian on our project and find places where duplicate code exists. We have pulled out a method in the PromptController and are working on multiple If statements in HuntController that have the same functionality. We ran across an issue with the original project where editing prompts doesn't actually work. This should be an easy fix but it is hard to test our changes in PromptController without it working.

Refactoring 2 - Reducing method sizes in domain classes

Though this wasn't a big change, we reduced the method sizes by changing how the code was written. I many places there was code written in longer forms that were not necessary and so we replaced them with the shorter and easier to read forms. We also changed non-descriptive variable names to ones that better explain what the variable does. There also was some functionality and logic that we ended up changing in the Notification spec.

Refactoring 3 - Renaming variables

We ended up just renaming variables as needed when simplifying methods in the domain classes. The impact was minimal but it helps to have something like 'photo' rather than 'p' for clarity.

Refactoring 4 - Refactor Large controller classes

We did not have time for this refactoring and will be moving it to the second iteration. We did extract methods but that fell more under the category of Duplicate Code rather than actually reducing the size of the class.