For example you write a code, which you want to improve later.
For now your simple function can solve the problem.
But later you want to use some library to calculate all.
// TODO: replace with vendor
function calculateTax($price, $country) {
return $price;
}
Is it a good idea to leave TODO comments in code and merge this code to master?
Here are few references
NO:
YES:
Github Action to Create Issues from Code TODOs
.github/workflows/todo.yml:
name: Create Issues from Code TODOs
# Controls when the workflow will run
on:
# Triggers the workflow on push events but only for the master branch
push:
branches: [main]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: TODO to Issue
uses: alstr/todo-to-issue-action@v4
Top comments (20)
Is it a good idea? Maybe not.
Is it the only thing I have time for because I can't spend hours or days achieving perfection when I already have something that's good enough? Frequently.
I think it depends on where the code is headed and whose eyes it's for. Yours? Leave them in, as long as it doesn't start to get crazy. What I usually do is I have my Markdown file with the longer-term things or overall goals that don't tie in directly to lines of code, and I move things in and out of there as they become more relevant/less relevant. For things that are much shorter-term, ie,
// TODO: fix the alignment hereor// TODO: figure out what the hell is happening, I leave those inline. Both of those are ones I'd take out of something I was handing in (student still) or showing as a portfolio thing.I feel like it's just fine to leave in things that are ideas or
TODOs that are more embellishment or realizing something isn't ideal but does work, because it shows some of your process--as long as it doesn't get too much, and you aren't passing this on to others who are going to need to slog through them. Something likeTODO: pull this into its own methodetc.But for anything that final, which hopefully there are fewer
TODOs in anyways, I would just keep a set/branch that has theTODOs in it, and remove them programmatically (regexes, awwwww yeah) for the final unless it's something another person is receiving and needs to do as a task.I really like this. This is basically what I do when I'm on a feature branch with intermediate commits, then part of my process for merging to
masteris filtering out the TODOs and making sure they're gone.I just did a search through the codebase for
TODOs, and found this gem from forever agoIn case you are using GitHub, Jason's bot might be useful. It creates GitHub issues corresponding to TODOs comment, so there is a less chance the todo would be forgotten, and you can discuss it better with your team ;)
I'll go ahead and say no.
Personally, I would prefer to have a more 'robust' way to keep track of things like that. By leaving comments like that, I think you run the risk of either forgetting about it, or losing the comment in some refactor or even having the TODO comment in a different part of the code, where it is not relevant.
Creating an issue on gitlab or github, or a card on trello, or whatever issue tracking system you may use in your team seems to be a better solution. A proper issue with a clear description of the problem to solve in the future will make everything clearer.
How would you show what exactly has to be changed (refactored) in code in issue?
Code and line numbers could be changed with a time.
Here is for example one suggestion to parse TODOs and create tickets automatically
TODOs bot does (mostly) that suggestion, for free hehehe
As the saying goes: "This is a topic upon which many intelligent people disagree"
Pros
I came to my current position a firm believer in both
FIXMEandTODOcomments in code. They seem entirely helpful, in that they're brain-dumps of the context you have in your mind while you're doing the code for tweaks not urgent, but important to deal with at some point.Also, back in the day, you might not have anything aside from version control, and in the worst cases, all you may have is a shared directory of code. The code was the only place to keep and track issues in context.
Last, people may say: "Log that in JIRA (or similar)" but not everybody reads those. Especially if you're working on F/OSS, people may not read the bug tracker, but they definitely read the code.
Cons
Yeah, sure. They "read the code" but what if there's a million lines? If I
grepa codebase and see 5000 TODOs I have no way to know what to tackle first.More bluntly:
TODOcomments get lost, get out-of-date, and they effectively become noise and technical debt themselves. My first checkin here had several 'TODOs' about things I noted in the codebase (algorithmic issues, internationalization problems, etc.) and the code review feedback was blunt: "Log these as issues in our bug tracker. Don't put these in the codebase."That's pretty much what I do now. If practical, a bug tracker with code links to the affected code surfaces issues much better from process perspective. Then, the TODOs can get prioritized and dealt with in your sprint.
There are two concerns here:
master.TODO/FIXMEto label that code.All else being equal, it's obviously better to have good and complete code over bad or incomplete one. But in real life, all else is rarely equal. The ideal code may need additional design and weeks of coding, or it may depend on something someone else should be doing and it'll be quite a while before they can do it - but your code needs to get in ASAP because you are halting development for everyone else and/or making all the integration tests fail on something trivial before they can get to the code they are actually supposed to test.
So, pragmatically, this is something you sometimes have to do. The second concern remains - are
TODOs andFIXMEs the right way to do this?Most of the opinions I've seen here - both in favor and against - seem to use a concept of "TODO as my bug tracker". The opposers claim that TODO is a terrible bug tracker and the "tickets" will just get lost. The supporters reply that proper tooling can help you manage the all the TODOs, and that having the "ticket" bundled with the code it refers to is an advantage both when creating and tending to it.
I, on the other hand, want to argue that TODO is not a bug tracker.
The things you mark with
TODO/FIXMEare not high priority. They are certainly not as high a priority as the code you had to commit even withTODOs andFIXMEs, and since the code does, somehow, works and functions, they tend to get lower priority than code that doesn't work and functionality that needs to be implemented.So, even if, in addition to the
TODO, you open a ticket in the actual bug tracker - you should not expect to get around to doing it that soon.So, the ticket will remain unattended in the tracker, but the
TODOcomment could still be encountered!Alice needs to refactor/fix/add functionality, and sees some weird code. She doesn't quite understand why it's there - "who is the imbecile that wrote it?". That imbecile may as well be Alice, because she hadn't recently touched that code, but even if it's Bob, he may not be able to answer - because maybe he hadn't touched that code recently.
But - if that code had a
TODO, explaining that it can be removed once the foobar bug is fixed - Alice doesn't even need to ask Bob. She knows the foobar bug was already fixed, and she now knows what's the meaning of that weird code - so she can safely remove it.Note that that
TODOis meant to be outdated - Bob was expecting people to encounter it after the foobar bug was fixed. He just couldn't get to it himself, because that code was not actually harmful and he had much more pressing things to do.So - TODO is not a bug tracker. You are not meant to get a list of TODOs, and prioritize them. It's a comment - it's meant to explain why the code looks that way and give you context if you want to change it.
Comments are lying. Those ones too. :)
if they exist only in your local branch I see no problem. I could pick any style of development you want. But when it goes to 'master/mainline/trunk' there should be no such thing. What if someone changed the code near that comment. He/she didn't do what's written in the TODO, and the change is substantial and this TODO doesn't make any sense. But the person changing it is to shy to remove your comment, because it is something you wanted to keep as a reminder for yourself, don't you? The TODO may become irrelevant because of changes in some other parts of the code. There's no way to track who's doing on what TODO comment. And from my personal experience in a large company you never have time to fix things afterwards, including those TODO comments, so they tend to stay in the code forever. I can easily find TODO comments in our codebase created in year 2001. (I think it's the year of import form the previous VCS) And they're all still there and nobody cares for them. :)
So, generally, I'd advise not to leave any TODO comments in the code. :)
There should be no such thing as a technical dept, but it exists in each project.
And as you noticed your old project there are a lot of todos, it doesn't mean that without those
todo comments the project would have more functionality. But I think it is better to write todos and ideas of what could be done rather than nothing. The solution not to have todos is to get it done. For this there should be a scheduled time to work on refactoring.
There are much better and much more reliable ways to track tasks you want to do on your code, that the code itself. :)
In the code you think you're in context of the surrounding code and everyone will understand what you're trying to say. Unfortunately, that's not true. The code tend to change and so does the context. When you enter the task in some external system (jira, trello, bugzilla, you name it), you need to provide all the details of what you have in mind. And yes, you can track it.
Still, there should be no TODOs in the code, they belong elsewhere.
BTW, I never though before, that putting TODO comments in the code is actually a way to hide what's need to be done. I believe no manager will look into the source code for the tasks to plan. :)
I like to leave TODOs in my code for small issues related directly to a bit of code. There are three good reasons:
1) For line-based issues, the TODO helps prevent "line drift". That is, if you state that you need to optimize the code on lines 32-67 (sometimes the best direction you can give), if you change the file significantly, lines 32-67 may become lines 108-143. The TODO helps anchor you to the location.
2) Other people (and the future version of myself) browsing the code may stumble across something that can be done immediately.
3) I'll try leave a TODO comment listing what I was last working on before putting the code away for the day/week/month, to help me resume work later.
That said, at work, we also follow The Rule Of Tasks:
Nearly every TODO should have an accompanying issue or ticket. What you don't want happening is your code becoming its own bug tracker! Again, TODO is good for short, code-specific issues, as well as for "I stopped here" reminders. For anything else, I don't recommend them.
P.S. If you're going to use
//TODO, you should also hook up your IDE or static analysis tools to detect and display them. Otherwise, you could seriously misplace them (in which case, you might as well have never put the comment in to begin with.) Atom, Code::Blocks, and cpplinter all handle this nicely.This is one of the few topics that I have a simple, and clean cut opinion on. But you know what they say about opinions...
If you're writing a TODO comment, that means you've written or discovered some code that needs some attention. So give it the attention. Now.
If you're writing something new, TODO's just shouldn't be a thing. Unless circumstances really restrict you, you should endeavour to write something cleanly and correctly the first time. If you were feeling out a problem and the code got messy along the way. Clean it up before you say you're "done".
If it's a small thing, like refactoring a function for readability or to match some particular code style. Just do it on the spot.
If it's a bigger issue that'll take some time to fix, it's time to start a short tech huddle to talk about the issue with some members of your team. From there a bug, tech task, github issue, sticky note, whatever should be created and prioritised with the rest of your work.
Every "TODO" comment is an in-code marker calling out technical debt.
They signify good intentions.
If the developers never go through and make a pass at cleaning up those TODO's, they'll accrue over time and slow development. (Even if the items are not called out with TODOs, the unclean code is still unclean. Just harder to find the filth without the TODO droppings.)
Making a conscientious effort to address the TODOs is important.
Clean Code by Robert Martin.
My project has TODOs that have lingered for decades. Yes, I said decades. /sadpanda