Is spreading code with refactoring comments a good idea?

Question

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?

related / duplicate: [Do TODO comments make sense?](http://programmers.stackexchange.com/questions/125320/do-todo-comments-make-sense) — gnat, Nov 05 '13 at 08:07
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 05 '13 at 08:08
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 05 '13 at 11:51

score 13 · Accepted Answer · answered Nov 05 '13 at 08:23

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.

score 2 · Answer 2 · edited May 23 '17 at 12:40

2

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 ;)

edited May 23 '17 at 12:40

Community

1

answered Nov 05 '13 at 11:14

Wolf

630
1
6
24

code snippet in the question reads like Java, why do you recommend Doxygen? – gnat Nov 05 '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 05 '13 at 13:54
1

@gnat: Do you think it's better now? – Wolf Nov 05 '13 at 14:06

Is spreading code with refactoring comments a good idea?

2 Answers2