Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Catch 22 - refactoring and writing a unit test

 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Report post to moderator
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.
 
author & internet detective
Posts: 41860
908
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Report post to moderator
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.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Report post to moderator
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.
 
Sheriff
Posts: 7001
6
Eclipse IDE Python C++ Debian Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Report post to moderator
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.
     
    author
    Posts: 14112
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Report post to moderator
    You should also take a look at http://groups.yahoo.com/group/welc/ for some tips.
     
    Ilja Preuss
    author
    Posts: 14112
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Report post to moderator
    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
    Sheriff
    Posts: 17644
    300
    Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Report post to moderator
    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
    Posts: 14112
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Report post to moderator

    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
    Sheriff
    Posts: 17644
    300
    Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Report post to moderator

    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
    Posts: 14112
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Report post to moderator
    OK, discussion continues here.
    I am closing this thread.
     
    Note to self: don't get into a fist fight with a cactus. Command this tiny ad to do it:
    a bit of art, as a gift, that will fit in a stocking
    https://gardener-gift.com
      Bookmark Topic Watch Topic
    • New Topic