Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am working on a "spaghetti-code" project, and while I am fixing bugs and implementing new features, I also do some refactoring in order to make the code unit-testable.

The code is often so tightly coupled or complicated that fixing a small bug would result in a lot of classes being rewritten. So I decided to draw a line somewhere in the code where I stop refactoring. To make this clear, I drop some comments in the code explaining the situation, like:

class RefactoredClass {
    private SingletonClass xyz;

    // I know SingletonClass is a Singleton, so I would not need to pass it here.
    // However, I would like to get rid of it in the future, so it is passed as a
    // parameter here to make this change easier later.
    public RefactoredClass(SingletonClass xyz) {
        this.xyz = xyz;
    }
}

Or, another piece of cake:

// This might be a good candidate to be refactored. The structure is like:
// Version String
//    |
//    +--> ...
//    |
//    +--> ...
//          |
//    ... and so on ...    
//
Map map = new HashMap<String, Map<String, Map<String, List<String>>>>();

Is this a good idea? What should I keep in mind when doing so?

share|improve this question
1  
related / duplicate: Do TODO comments make sense? –  gnat Nov 5 '13 at 8:07
2  
This is an opinion-based topic; but my personal opinion is that this is exactly the type of comment that is useful, and that I wish would find in other people's code: it tells you important information that isn't already obvious from the code; not what a method does, but why. –  Kilian Foth Nov 5 '13 at 8:08
2  
HashMap<String, Map<String, Map<String, List<String>>>> :o –  margabit Nov 5 '13 at 11:23
4  
Comments that tell me why a piece of code looks smelly are incredibly hugely appreciated. I may not have your understanding of the codebase, so I'm just gonna see an issue and think "What the fuck?", but a comment explaining why it is as it is will help me get around the code faster. Yes, very much do this. (Assuming you can't fix the code to not be WTF, of course!) –  Phoshi Nov 5 '13 at 11:51

2 Answers 2

up vote 13 down vote accepted

Is spreading code with refactoring comments a good idea?

If you have allocated time to finish your refactoring, and if you really do it, then yes - it will work out.

What should I keep in mind when doing so?

Modern IDEs have an option to find and show TODO lines. You should check them from time to time, and try to reduce their numbers whenever you can.

share|improve this answer

I would make such considerations /// @todo comments for doxygen or an easy-to-install custom tag for javadoc, so it gets automatically extracted to the todo section of the API docs. Plain comments will be overlooked too easily and eventually get lost in the depths of code.


[Edit] BTW: is this a good idea:

while I am fixing bugs and implementing new features, I also do some refactoring in order to make the code unit-testable

I think (know by experience!), refactoring can be very dangerous, especially when there are still no unit tests. So you'd better restrict your extra work (when fixing bugs etc.) on adding todo comments ...We all know: when ever possible ;)

share|improve this answer
    
code snippet in the question reads like Java, why do you recommend Doxygen? –  gnat Nov 5 '13 at 13:35
    
I did know that doxygen supports @todo - for javadoc I wasn't sure - but is the language really so important? From my point of view, the java example illustrated a deeper problem. –  Wolf Nov 5 '13 at 13:54
1  
@gnat: Do you think it's better now? –  Wolf Nov 5 '13 at 14:06

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.