Sometimes technical debt is unavoidable. There are lots of good reason why the code you have today is increasing your maintenance costs and slowing feature development. Sometimes you have to accept a hack in order to ship (shipping is a feature). Or if your product has been around a while, it's probably evolved far beyond the original design, and it just needs to be restructured. Or maybe it's not a good reason but it happened anyways, e.g., you trusted a less than stellar developer, forgot to write unit tests, or adopted a framework that sounds great in theory but just doesn't make any sense in practice. In any case, taking an iteration or more (when you can afford it) to deal with technical debt is just par for the course.
The first mistake made by many developers when they start refactoring is to start by cleaning up code. That way is a rabbit hole that will take you through multiple iterations to increasingly madder tea-parties (read: standups) and end with your PM screaming "Off with his head!" because you've made the codebase unworkable for weeks, all in an effort to make things better. It starts so innocently, with a single class. Then you notice that the problem with that class is related to one of its dependencies, so you chase that dependency down and discover that the general architecture is too small (or too large) so you eat the mushroom or drink the drink and soon you're drowning in your own tears. The caterpillar may console you momentarily by giving you a hit off his hookah, but it's just going to get weirder. Better to stay out of the rabbit hole.
The key is limiting the scope and hammering it into everyone involved that "WE DO NOT TOUCH ANYTHING OUT OF SCOPE". I'll say that again, in all caps WE DO NOT TOUCH ANYTHING OUT OF SCOPE. "But, but, the service layer is still a mess, which is why w-" WE DO NOT TOUCH ANYTHING OUT OF SCOPE.
The first step in a successful refactoring is for a senior developer or lead to sit down and figure out the smallest possible refactoring that can be done. Then create specific instructions on how the refactoring should be done. Literally, instructions like "Move line X to class Y and put Z where X was." You don't want to leave it up to the developer to come up with their own way of doing things. Developers get bored easily, and a single developer can try six different ways of doing things before he decides on one he likes.
Once you have detailed instructions and guidelines, cut your developers loose. After the first commit, be prepared to devote at least 10 minutes of your next standup to explaining the instructions again, because it turns out half of them didn't read them. It's not that you have stupid or bad coders on your team, it's that they're smart and think they don't need instructions. It's more fun to get coding and figure it out as you go, right?
Because you made the original refactoring small, once the effort is underway, you need to start planning the next one. Small and iterative works for features and it works for bugs. It will work for refactoring just as well. Don't waterfall refactor.
It goes against every technical design instinct we have, but the better course of action will almost always be to incrementally improves components of a system rather than redesign it from the ground up. You also have to keep the app running. If HEAD is unusable for even a day because of your refactoring, you're trying to change too much at once. Sometimes that means writing some glue code that is ugly and inelegant so that the component you're refactoring can be pretty and elegant. Your inner architecture astronaut will be screaming as he burns up on rentry, but even NASA sometimes needs duct tape.
Here is an example to get you started: Take your JSP/ASP/GSP or PHP templates. Define model classes for them, based on the needs of the UI, not based on whatever model you have in the database or ORM or the DTOs your services return. You should end up with simple inline expressions. e.g., ${model.foo} rather than ${_POST['foo'].replaceAll("\\.","-")} or ${fooList[i].bar.baz.foo}. Move all that ugly logic into your controller. That part is easy. The hard part is resisting the urge to go back your service layer or even your ORM mapper and develop a factory and mapping system that trasforms the output to perfectly match your templates so you can just write Foo.list(params) in your controller and have it be oh-so-elegant. It will be done in 2012. Remember: Worse is better.
Showing posts with label refactoring. Show all posts
Showing posts with label refactoring. Show all posts
Wednesday
'Opt-In' Changes and the Importance of Testing Suites
A development mistake I see all the time is when a developer fails to make changes to core classes 'Opt-In' or backwards compatible. For example, suppose you find a bug in your table/grid widget that causes one of the several instances of that widget to fail mysteriously. You come up with a solution that involves an extra method call or two at a certain point, or changing an option/property to another value. Your first instinct might be to make that change the default behavior, but in most cases, that's wrong.
Summary: How many instances of that widget do you have in your application? Are they all failing? Why not? Were they functioning properly before? If they ain't broke, why fix 'em?
Most likely the change you are going to make really should have been the default behavior all along, however, you now have working code that was developed and presumably tested against the old behavior. If you change that behavior now, even if it is now the 'right' behavior and was previously the 'wrong' behavior, you now need to test every single instance and subclass of your widget in all possible scenarios to ensure you haven't introduced a bug. Do you have a comprehensive automated testing suite? If you do, great, but you probably don't.
Even if you do have a good test suite, why fix something that isn't broken? Suppose, instead of making that change the default, you instead add a
Now some people will learn of problems like this and say you should have done more testing before committing. You know, you should have gone through all the major paths of your application to ensure that they're working before committing. Most of the time those people are managers who don't have to waste time doing the testing themselves, but expect you to do it.
Suppose a developer spends X time testing before a check-in and finds some bugs that take Y time to fix, it has cost him X+Y time. However if he instead verifies that his code does what he intended it to do and checks in, but someone discovers the bugs, emails him, and he spends Y time fixing it, it only cost him Y time. Now if X is small, say 30 seconds, because all the developer had to do was kick off the automated testing suite, then the developer will do the testing to avoid getting angry emails. However, if testing takes more than a few minutes, most people will realize they can get more work done by letting someone else hunt for bugs. You can argue about it all you want, you're not going to change human nature, and the optimal choice most of the time is to skimp on the testing.
In short, you have to work with people the way they are. No amount of training or lecturing will keep people doing things that are not in their own best interest. If your project's big problem is that people keep breaking the build, make not breaking the build the easy path. You may have to sacrifice some hours to developing and maintaining a test suite, but you will generally save more hours of lost productivity from broken builds.
Minimize Affect
Summary: How many instances of that widget do you have in your application? Are they all failing? Why not? Were they functioning properly before? If they ain't broke, why fix 'em?
Most likely the change you are going to make really should have been the default behavior all along, however, you now have working code that was developed and presumably tested against the old behavior. If you change that behavior now, even if it is now the 'right' behavior and was previously the 'wrong' behavior, you now need to test every single instance and subclass of your widget in all possible scenarios to ensure you haven't introduced a bug. Do you have a comprehensive automated testing suite? If you do, great, but you probably don't.
Even if you do have a good test suite, why fix something that isn't broken? Suppose, instead of making that change the default, you instead add a
useFooHack property to your base class. It defaults to false, but if it's true, you use your new behavior. When you discover that other instances have a problem caused by the old behavior, you just change the flag to enable Foo, and it's fixed. Easy as that. If you discover that every last instance has the problem, you can remove the useFooHack=true line from your instances and if(useFooHack) from your base class. Simple. The alternative is to risk introducing bugs into every instance that you thought was working.Testing is a waste of time
Now some people will learn of problems like this and say you should have done more testing before committing. You know, you should have gone through all the major paths of your application to ensure that they're working before committing. Most of the time those people are managers who don't have to waste time doing the testing themselves, but expect you to do it.
Suppose a developer spends X time testing before a check-in and finds some bugs that take Y time to fix, it has cost him X+Y time. However if he instead verifies that his code does what he intended it to do and checks in, but someone discovers the bugs, emails him, and he spends Y time fixing it, it only cost him Y time. Now if X is small, say 30 seconds, because all the developer had to do was kick off the automated testing suite, then the developer will do the testing to avoid getting angry emails. However, if testing takes more than a few minutes, most people will realize they can get more work done by letting someone else hunt for bugs. You can argue about it all you want, you're not going to change human nature, and the optimal choice most of the time is to skimp on the testing.
In short, you have to work with people the way they are. No amount of training or lecturing will keep people doing things that are not in their own best interest. If your project's big problem is that people keep breaking the build, make not breaking the build the easy path. You may have to sacrifice some hours to developing and maintaining a test suite, but you will generally save more hours of lost productivity from broken builds.
Subscribe to:
Posts (Atom)