There appears to be this notion among the project management team that stating that "it works" means it should then be deemed 100% complete. Most programmers know that isn't always the case. If I'm trying alternative approaches to get a piece of functionality working, that doesn't necessarily mean I found the best solution, or it won't require some rework after reviewing with other developers. I'll often get done with something, step back, and then ask myself what I can do better after the business rules are satisfied. Should this "I can do better" time actually fit somewhere within the timeline? I'm of the opinion that the best approach is that you always leave code better than when you found it (to a degree), which could mean post launch refactoring. However, project teams are often extremely uncomfortable with this approach because again, if it works, and has been tested, then why fix it?
7 Answers
There is one overarching principle that governs the need to refactor and optimize, both in waterfall and Agile: YAGNI (You Ain't Gonna Need It). A second principle is the corollary of the first: "Premature optimization is the root of all evil", the coding equivalent of the general proverb "the enemy of excellence is perfection".
Let's take the priciples and apply them. You have the requirement to build an ETL algorithm that takes a file of a particular type, extracts its information, then puts that information into a database. Your goal for this week (for our purposes it doesn't matter whether you're in an Agile or SDLC shop) is to get that done.
You're a smart fellow, and you have been given a glimpse of the big picture. You know that this is not the only type of file for which the project will need an ETL. So, you consider implementing this ETL algorithm to also work on another type of file, which has only minor differences. Doing this would violate YAGNI. Your job is not to develop the algorithm for that other file; it is to develop the algorithm for the one file that is needed by the end of the week. To meet that goal and pass the acceptance tests, you need to develop that algorithm and make it work correctly. You "ain't gonna need" the additional code to make it work with the other file. You may think it will save you time to incorporate it now, and you might be right, but you might also be terribly wrong; the algorithm for the other file might need to be used in an area of the system your code can't be used, or the requirements for the new file might be different than for yours in ways you don't know (in Agile, those requirements may not exist yet). In the meantime, you've wasted time and unnecessarily increased the complexity of your algorithm.
Now, it's next week, and as a dubious reward for your excellent work on the first algorithm, you have been given the task of creating the algorithms for two new file types. Now, you DO need additional code to make your algorithm work with more files. You may extend your existing algorithm using a template method pattern that will use a basic pattern with file-specific individual steps, or you may simply derive a common interface from your existing algorithm, develop two new ones that follow the interface, and plug them into an object that can choose which algorithm to use.
While developing, you know you have a requirement that the system be able to process 10KB of raw data per second. You do a load test and find your initial draft algorithm handles 8KB/s. Well, that's not going to pass the AATs. You take a look and see that there's some O(my God)-complexity loop structure in your algorithm; you streamline it and get 12KB/s. "Pretty good", you think, "but if I had that poor a loop in the code, what else can I shave off?". buzz You just violated the "premature optimization" rule. Your code works, and passes all requirements. You're "done", until such time as the requirements are updated to require 15KB/s. If and when that happens, THEN you pull the code back up and look for things to improve.
Follow this simple process while developing, whether in Agile or in traditional SDLCs: "On the first pass, make it work. On the second pass, make it pretty. On the third pass, make it SOLID." What this means is, when you first create a line of code, make that code do its job correctly and bug-free, but don't pay too much attention to design rules within this code, as for all you know right now you'll never touch this area again. The next time you visit that line of code, you just proved yourself wrong; it's no longer a one-off piece of the system. Refactor it for readability, conciseness of code, and/or DRY principles (you may have copy-pasted some code to do something five times; refactor that into a loop and/or a method call). The third time you are working in or around that line of code, it should be considered a key area of the system, and you should refactor it for SOLID structure, perform easy-to-spot optimization enhancements, etc. Follow this, and you'll find that the areas that need refactoring receive it, but at the same time no time is wasted.
-
3+1 for
O(my God)-complexity
if nothing else, made me laugh!– Joel CCommented Jul 12, 2011 at 19:19 -
+1 for Get it working first. Too many people try to write patterns and prematurely optimize off the bat. Commented Jul 13, 2011 at 2:18
-
I find this to be one of the most difficult issues to overcome as a programmer. Engineers have an innate desire to experiment, develop, and refine, but at the end of the day, you are getting paid for productivity. What good is a perfect product if you spend so much time and/or money that it's canceled due to overruns?– ToddRCommented Jul 13, 2011 at 12:34
-
I like the pragmatic aproach but I have a problem with "On the second pass, make it pretty": if the 2nd pass is one year later and you havent make shure that variable and method names are meaningfull and magic numbers were replaced by symbolic constants you probably have problems to understand the code. How to deal with this? "Make it pretty" 1 hour after "make it work" is much much cheaper than "make it pretty" after a month or after a year. I agree that "make it pretty" when next code change is neccessary is usefull if "make it pretty" has not been done in the first place.– k3bCommented Apr 11, 2017 at 10:24
-
In Agile/TDD, it's been my experience that the second pass typically happens pretty soon after the first. In Waterfall-ish SLDCs, you're more right; the function tends to get written once and then sits there until the next round of requirements comes through that touches that method. So, some good coding practices need to happen the first time around, like self-documenting code, so that when you come back a year later, you can remember what the code does and why you wrote it that way.– KeithSCommented Apr 11, 2017 at 15:25
if it works, and has been tested, then why fix it?
This may go against your own personal temperament as an engineer/programmer, but if it's working, what business value is there for you to continue refining it? Will it make it easier to maintain over time? If so, then working under the agile methodology, you should be able to create new items in your backlog to refine and refactor your existing code, and those would get prioritized with the other items in the backlog. That's part of the value of the agile process: the team decides together what's most important and what gets done next.
Our team also tracks what we call "technical debt", so if you get something working but you know it could be done better, you log it as technical debt. We use scrum, and sometimes you'll finish all the work in a sprint early (you should finish a little early roughly half the time if you're fairly close with estimates), but not have enough time left to pull in a completely new user story, so we spend the extra time going back and working down our technical debt. It's not tracked formally like our user stories in our backlog, and we can pretty much work on it whenever we have time available.
It's also more or less a judgment call on your part when you call the task "done"; if you're not comfortable with the state your code is in, don't mark the task complete.
-
2I've become a fan of the "technical debt" concept, +1 for bringing it up in this context. Commented Jul 12, 2011 at 17:07
-
Completely forgot about the idea of "technical debt"; good term. But, I was taught that anything that qualified as "technical debt", meaning it would require significant developer cycles to refactor, was to be avoided; "do it light" still meant "do it right", just don't go "ivory-tower" on code that may be a one-off.– KeithSCommented Jul 12, 2011 at 22:55
Should this "I can do better" time actually fit somewhere within the timeline?
Yes.
Just before you start coding the next release.
Don't refactor based on "intuition".
Refactor based on the next sprint's actual stories.
Don't mark the code as 100% complete until you are satisfied with the refactoring. You just have to constantly evaluate the cost/benefit to refactoring the code because if you study enough you will always see ways to make the code better.
I use the red green refactor method of TDD. So my refactoring is built into my development. For big refactorings like changing the underlying model or something similar I would get management buy in to spend the time first.
-
1Code is not "100% complete" until all products in which it resides are dead. Much like you are not "complete" as a person until your heart stops beating permanently; you will always absorb new information and be required to do specific things you have never done before, or do the same thing in a new more efficient or less expensive way. Similarly, your codebase will ALWAYS need work - new features and old fixes - until nobody uses the software anymore.– KeithSCommented Jul 12, 2011 at 17:24
"post launch refactoring" has a hidden cost in regression testing and QA time that you are ignoring, plus it carries the opportunity cost of not working on reported bugs and new/requested features and changes. TANSTAAFL
If it is worth doing, it is worth making a task to get prioritized via your normal process and not as a special exception. You are part of a team, after all, and working on common goals and arbitrarily extending your schedule to accommodate fixing working code impacts them too.
So for a real answer: if you know that you'll want to refactor then schedule that time as part of the task. If you're doing scrum/agile then time box a clean-up task. If you're waterfall/spiral then make the refactor part of the process to code review and accept the modules.
If I'm trying alternative approaches to get a piece of functionality working, that doesn't necessarily mean I found the best solution,
...In which case you are not 100% done yet...
or it won't require some rework after reviewing with other developers.
If code reviews and subsequent rework is part of your development lifecycle, again the feature is not done until all of these are finished.
I'll often get done with something, step back, and then ask myself what I can do better after the business rules are satisfied. Should this "I can do better" time actually fit somewhere within the timeline?
It depends. If it means refactoring, it should be part of the original development task. If it means experimenting with a potentially better algorithm, it could be a separate task.
I'm of the opinion that the best approach is that you always leave code better than when you found it (to a degree), which could mean post launch refactoring. However, project teams are often extremely uncomfortable with this approach because again, if it works, and has been tested, then why fix it?
In brief, because code can be broken on many levels.
It is one thing that it works right now. It is an entirely different thing whether it is clean, extendable and maintainable in the long run.
For more detailed answers, see this thread.
As far as I can see and have read, this is an unsettled question. Hence, the avoidance answers like "YAGNI" and do-it-right-the-first-time responses. The fact is there isn't a place in Agile for refactoring -- but I would argue that there should be.
The best answer so far mentions technical debt. This, unfortunately, is a sad reality of software in many enterprises, where the rush to get things out-the-door whether in an agile or non-agile methodology is all to common -- but under Agile the quick-and-dirty solutions are rationalized as being something good: "meets the minimum business requirement" and "YAGNI" (with regard to keeping code clean).
It would be great if everyone did TDD, and it would be great if all developers refactored the second or third time around as suggested by one answer. But that just doesn't happen in the real world. Developers of varying skill levels nearly always are found to have cut corners in the pursuit of quick solutions. As a result the code decays into mountains of unmaintainable code that takes new developers days just to decipher, which hurts productivity and delays deadlines. By "unmaintainable" I mean copy-and-paste solutions, 5000 line classes etc. And all this code and these fixes upon fixes are all core to the business! -- In these cases of additive solutions, I would argue, there's no such thing as YAGNI! You are going to need clean code -- ALWAYS. If the code is not clean you definitely will-not-need-it -- see the self-fulfilling prophecy? Developers would go to great lengths to not use that code at all because it's too painful to look at. And the vicious cycle goes on and on until the whole big-ball-of-mud has to get chucked and rewritten.
So I say -- despite code-refactoring not being a proper, distinct, worthy-of-it's-own-story-type Agile concept -- we should make time to refactor. Some shops now are requiring teams to spend 20% of their sprints on technical debt. Hopefully, the agile proponents will change their minds about YAGNI and make a place for refactoring as a separate time-allotted activity. And if they have already and I haven't heard of it, please point to where that's described because I'm very interested to know.
-
" The fact is there isn't a place in Agile for refactoring" - I don't think that's a true statement. In agile, there's a place for any sort of development, including refactoring, as long as there's a business case for it. If there isn't a business case for it, why are you doing it? Commented Apr 10, 2017 at 16:27
-
I guess you have a point if a bit simplistic. In theory a developer could fabricate a business case for fixing shoddy code even if it doesn't produce any functional changes, but that wouldn't jive with the spirit of agile -- using business as a proxy to justify work. I would argue then that the activity of refactoring lies outside the realm of agile -- a sort of CYA activity if-you-will -- fixing unmaintainable code so it doesn't cost the business long term and the developers are blamed. Call it a "refactoring sprint" or whatever, but there must be a formal place for it. Commented Apr 10, 2017 at 22:26