wood burning stoves 2.0*
The moose likes OO, Patterns, UML and Refactoring and the fly likes Catch 22 - refactoring and writing a unit test Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


JavaRanch » Java Forums » Engineering » OO, Patterns, UML and Refactoring
Reply locked New topic
Author

Catch 22 - refactoring and writing a unit test

Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4474
    
    6

A class that I have been working on recently has lead to, as the ancient Chinese curse goes, "very interesting times" for me. A number of code smells are apparent in it including Duplicate Code, Long Method, Verb-Subject methods, and Law of Demeter violations to name just a few.
Now I am tasked to write a unit test for this class so that it's easier to detect if something in it is broken.
This brings me to my Catch-22 situation. To write a unit test, I need to be able to refactor with confidence. But to refactor with confidence, I need a unit test. So where and how the heck do I start? I realize that it's difficult to retrofit a class with a unit test but this is something that must be done somehow.
First step it seems to me, would be to loosen the coupling between this class and it's many collaborators. This isn't going to be easy because it's a class that sits between the presentation and database layers of the application (it's a Struts-based application and this class is called the InboxTaskProcessor, a "Manager" type class). It has direct references to concrete classes (as opposed to abstract classes or interfaces) on both sides of the fence.
If you have encountered this kind of problem before, any suggestions or tips on how to proceed would be greatly appreciated.


Junilu - [How to Ask Questions] [How to Answer Questions]
Jeanne Boyarsky
author & internet detective
Marshal

Joined: May 26, 2003
Posts: 30580
    
154

Junilu,
While mock objects are more useful if you have interfaces, they can be used to simplify your situation. You can create mock object type classes that subclass the troublemakers in the method. In your method under test, replace new MyObject() with getMyObject(). Obviously, this new method just returns MyObject. This is a fairly safe refactoring. When you test, you can override this new method to return your mock type object. I know this is a lot to do, but it's a pretty safe way to yank the code out.
Alternatively, you could start with an integration type unit test (that really accesses the db) to provide full test coverage. Then, it would be perfectly safe to refactor.


[Blog] [JavaRanch FAQ] [How To Ask Questions The Smart Way] [Book Promos]
Blogging on Certs: SCEA Part 1, Part 2 & 3, Core Spring 3, OCAJP, OCPJP beta, TOGAF part 1 and part 2
Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4474
    
    6

Yes, what you suggest seems like a good place to start. Eventually, I think refactoring this class so that I can do IoC with it will make it more testable. Adding the getCollaborator methods as you suggested would be a step towards this goal.
Thanks, Jeanne.
Frank Carver
Sheriff

Joined: Jan 07, 1999
Posts: 6920
Junilu writes: {i]To write a unit test, I need to be able to refactor with confidence.[/i]
I can understand why you feel this, but I'm not sure I really believe it.
Step One in this situation is to write some tests for the existing API to this ball of mud. However clumsy these tests may be (long smelly setups, slow tests, manual resets, whatever) they are where you need to begin. Without this, you are in grave danger of getting to much rewriting grit in your refactoring gears.
The importance of providing tests for teh existing API are that:
  • they ring-fence the area you are working on, so you are less likely to keep expanding your rewriting efforts to encompass larger and larger areas of the code - a process which always takes way too long.
  • they ensure that you really understand what the code currently does, and how existing client code uses it. In any major rewrite, mistaken assumptions are your biggest danger.
  • they give you a concrete jumping-off point for your rewriting. Fix each smell in the clumsy tests, one by one, and your code will improve to the point where more detailled unit tests are possible. Without this anchor it is far to easy to spend a lot of time tidying and polishing code which subsequently gets discarded or completely rewritten. Focus is vital.


  • I hope some of this helps.


    Read about me at frankcarver.me ~ Raspberry Alpha Omega ~ Frank's Punchbarrel Blog
    Ilja Preuss
    author
    Sheriff

    Joined: Jul 11, 2001
    Posts: 14112
    You should also take a look at http://groups.yahoo.com/group/welc/ for some tips.


    The soul is dyed the color of its thoughts. Think only on those things that are in line with your principles and can bear the light of day. The content of your character is your choice. Day by day, what you do is who you become. Your integrity is your destiny - it is the light that guides your way. - Heraclitus
    Ilja Preuss
    author
    Sheriff

    Joined: Jul 11, 2001
    Posts: 14112
    By the way, you shouldn't expect the class to work correctly - complex classes often have some very interesting bugs. On the other hand, some clients might actually be *depending* on existing bugs...
    As I think about it, I tend to more and more agree with Frank (I think). You should be very wary of the refactoring becoming your "silent goal". Your task is to write tests for the class. Of course you will probably need to do *some* refactorings to be able to do that effectively, but the class doesn't need to be perfect when you are finished.
    The best way to do a big refactoring is to not do it, but to do many small ones, parallel to increasing business value.
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4474
        
        6

    Frank,
    You hit on one of my fears going in: writing smelly tests. OK, if this is one of the sins I must commit before making things better, so be it.
    If you folks don't mind, I would like to keep posting to this thread to record my progress and further solicit comments/suggestions.
    Ilja Preuss
    author
    Sheriff

    Joined: Jul 11, 2001
    Posts: 14112
    Originally posted by Junilu Lacar:
    You hit on one of my fears going in: writing smelly tests. OK, if this is one of the sins I must commit before making things better, so be it.

    On the other hand, perhaps you should concentrate more on acceptance tests than on unit tests?
    Can you tell us more about why this particular class was choosen and what system(s) it is used in?
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4474
        
        6

    Originally posted by Ilja Preuss:
    On the other hand, perhaps you should concentrate more on acceptance tests than on unit tests?

    That would have been my next question. Right now I feel, as our trailboss would say, "icky" from the tests I've written so far. Since the class has so many collaborations, I've had to copy-paste a lot of code from those classes just to get the class working from the test case. That has to be a code smell of some sort.
    Originally posted by Ilja Preuss:
    Can you tell us more about why this particular class was choosen and what system(s) it is used in?

    The class is a rather length one (over 2000 LOCs) with large methods and a lot of duplication. It's complex and hard to understand and it has been the source of bugs/errors in the (manual) testing phase.
    I'm trying to write automated tests to reveal the bugs that are being found. Right now I'm using JUnit but if there is another testing tool that would be more appropriate to use for this situation, please tell me.
    This class will eventually need to undergo major refactoring/rewriting but for now we just want to make sure that it satisfies requirements.
    I think the discussion is turning more towards testing so I will continue this in a thread in the Testing forum.
    Thanks!
    Ilja Preuss
    author
    Sheriff

    Joined: Jul 11, 2001
    Posts: 14112
    OK, discussion continues here.
    I am closing this thread.
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Catch 22 - refactoring and writing a unit test