Refactoring

Editor's Notes

• #12 RearchitectingAs you increase your knowledge of the code, making it more comprehensible and better factored, you'll inevitably find some bits you don't like. When you started this project, you weren't really capable of changing much, but now you've played with the code, and it's not as damaged as it once was. Along the way, you have seen some bad things and had some ideas on how to fix them. It's time to make some serious changes. Rewrite code you don't understandIf you know what the code should do, and it seems to do it, but you can't figure out quite how, it's bad code. If the original author did something tricky, some comments should have been provided. More likely though, the author was clueless and the code really is a disaster. Rewrite it. By rewriting the code, you have the opportunity to do it the easy way, and to include useful comments. Start by writing down in Javadoc comments what the new code does (just to make sure that you know). Then you can take advantage of the byproducts of your refactoring: call this new method that does that, use that new data structure to store this. The new code will make more sense to everybody and will probably run faster as well. Your unit tests will tell you whether your new code works. Go into your bug tracking system and close all the bugs you have fixed. Move to a layered architectureLet's hope that your inherited, formerly bad code now looks better. It's time to look at the bigger picture: how the packages relate to each other. Using some sort of tool that tells you what classes invoke what methods (that information can be extracted from the class files), find out the interclass dependencies. From those dependencies, infer the interpackage dependencies. If you can't draw a hierarchy (without any loops!) of what packages depend on what other packages, you have some architectural work to do. Using your instincts, a code browser, and a great deal of guesswork, figure out what such an architecture might look like one fine day. Then identify the particular problem spots. A particular class that frequently refers to other packages or a particular constant referred to from other packages hints that classes or parts of classes reside in the wrong package. Ask yourself whether it would make sense to move that code to another package. After a lot of consideration, you can derive a package hierarchy that looks sensible. Although it may require a lot of work to achieve the clean architecture, it gives you a goal to work toward. Your unit tests have clarified what the code should do, and now the architecture allocates the responsibility to do it to packages and classes. All you have to do is fill in the code.
• #18 Break up big methodsJust as big classes prove difficult to understand, so do big methods, whose usual causes include:Too many options, causing the method to do too many thingsNot enough support from other methods, causing the method to do tasks at a lower level than it shouldOverly complicated exception handlingMy rule of thumb for methods: If I can't fit the entire thing on one screen, it's too long. It's an entirely practical rule with no theoretical basis, but it seems right and works for me. Remember though, when you read a method of 100 statements for the first time, it looks like 100 unrelated statements, and it is only after some study that you can see the internal structure. Life would be easier for you and for others if you made that internal structure explicit.
• #20 Fix the Javadoc commentsYou never realize a comment's importance until you need to read it. Even if nobody else reads it (and usually, they don't), Javadoc comments are important for the code authors to help them remember what the code/class/parameter should do. Pieces of information particularly important to include are: Whether an object parameter may be null, and what it means if it isWhether a parameter needs to be mutable or notWhether a return value is mutableWhether a return value may be nullWhether changes to the return value affect the returner's internal stateFixing the Javadoc comments doesn't mean just adding them where there were none before. You should also delete misleading and trivial comments (such as documenting setters and getters) as they engender a false sense of security. Important information exists for inclusion in Javadoc comments, so there's no point wasting the time of potential readers. When you write new methods and classes, try to include some Javadoc comments (just a few) to explain what they do. Even if it's obvious to you, the next person to get the code might thank you for the effort. If you don't know what to write in the Javadoc comments, you have no business writing the code. Javadoc comments can also serve as a record of what you learned about the code. When you figure out what a particularly tricky, clever, or ugly method does (especially a private or protected method with no implied contract), write a comment to record it for posterity. This expands the amount of the code under your control and helps you later when you're trying to decipher something related. Of course, Javadoc comments should only explain the effect of a method, never the implementation (because you might change that). If code within a method needs explanation, use standard code comments. A school of thought exists that says with good enough identifiers you don't need code comments at all. I'm not that extreme, but I take the point. Indeed, many code comments don't tell you anything new. I urge you to leave those uninformative comments out, as they only serve to make simple code verbose and intimidating.