Summary
If you are like most, and possibly even all, programmers (this
author humblely included) then your code sucks. Maybe not all of it, and maybe not all the time, but certainly some of it, some of the time.
Advertisement
Your code sucks if it doesn't work.
Does your code work? How do you know? Can you prove it?
If you don't have tests for your code, it sucks. And I mean
comprehensive, fine grained, programmer tests (aka something like unit
tests), as well as higher level functional and integration tests.
Tests that are automated. Tests that are run routinely. With the
programmer tests run after any change to the code.
Without adequate tests (in terms of both quantity and quality) you
simply can't have confidence that your codes works as required. And
even if you are confident with your work... why should anyone else be?
What about people who work on the code later? How do they know it
still works after they make their changes?
Your code sucks if it isn't testable.
OK, so you decide to add tests to your code. That typically isn't
so easy. The odds are that your code wasn't designed and/or
implemented to be easily testable. That means you will have to make
changes to it to make it testable... without breaking any of it (this
is usually called refactoring these days). But how can you
quickly and easily tell if you've broken anything without that
comprehensive set of fine grained tests? Tricky.
It's even trickier than that. If you take the approach that you'll
write tests for all your code after you write the code, you
have to go to extra effort to try and design/write the code to be
testable.
Maintaining an adequate level of testing when writing tests after
the code being tested takes time and planning. And if you have to do
additional work to make your code testable... well... that sucks.
Writing tests before you write the code means that the code is
testable by definition. If you work one test at a time, you reap even
more benefit, as each test only requires incremental changes to code
that you know is solid.
Your code sucks if it's hard to read.
Code should be easy to read. Steve McConnell made the
statement at his SD West '04 keynote that code should be convienent to
read, not convienent to write.
There are several things involved here. Choosing good names that
are self-explainatory is a good place to start. Strive for simple
solutions, even if they are more verbose or inefficient. Whether
inefficiency is a problem won't be known until profiling is done later
in the project, in a real production situation.
Choose style and formating conventions early in the project, and
conform to them. Require that everyone conform to them. This will be
easier to do if everyone has a hand in deciding on the conventions and
buys into them. This will make the code uniform and thus easier to
read.
And get rid of any and all duplication. More on this later.
Your code sucks if it's not understandable.
This is closely related to the previous point. Code can be
readable, but still not easily understood. Maybe it reads well but is
misleading. Maybe you took the time to pick a good name for that
variable, but the way it's used and what it means has changed, but the
name hasn't. That's worse than using a cryptic name... at least in
the latter case, the reader knows the name is meaningless. If the
name is missleading, the reader can make ungrounded assumptions about
what's happening in the code, leading to misunderstanding, and
eventually bugs.
Your code sucks if it dogmatically conforms to a trendy framework
at the cost of following good design/implimentation practices.
For example, Bob Martin recently raised the issue of dogmatically
using private fields and getters/setters for a simple data structure
(e.g. a DTO). If a field is transparently readable and writable why
not simply make the field public? In most languages you can do that.
Granted, in some you can't. For example, traditionally in Smalltalk
all fields are private and all methods are public.
In general it's a good thing whenever you can throw out, or avoid
writing, some code. Using a heavy framework generally requires that
you must write a significant amount of code that has no business
value.
There are a variety of lightweight frameworks for Java that are a
response to the heavyweight frameworks (e.g. EJB) that have
become matters of dogma lately. O'Reilly has a new book out on this
topic, coauthored by Bruce Tate.
When making framework decisions, consider if a lighter framework will
do the required job. Using something like Hibernate, Prevayler,
Spring, PicoContainer, NakedObjects, etc. can be a real win in many
situations. Never blindly adopt a heavy framework just because it's
the current bandwagon. Likewise, don't blindly adopt a lightweight
framework in defiance. Always give due consideration to your
choices.
Your code sucks if it has duplication.
Duplication is probably the single, most important thing to banish
from your code.
Duplication can take several forms:
textual
This is the simplest and often the easiest
to find. This is when you have sections of code that are identical or
nearly so. This is often a result of copy & paste programming.
Several techniques can be used to eliminate it, including: extracting
methods and reusing them, making a method into a template method,
pulling it up into a superclass, and providing the variations in
subclasses, and other basic refactorings. See Fowler's "Refactoring"
for more detail.
functional
This is a special case of textual
duplication and is present when you have multiple functions/methods
that serve the same purspose. This is often seen in projects that are
split up between groups that don't communicate enough. This could be
a result of copy & paste, or it could be the result of ignorance
of existing code or libraries. Practices and policies that encourage
and promote knowledge of the entire codebase and all libraries and
frameworks being used can help avoid functional duplication.
temporal
This is a bit odder, more obscure, and not as
easy to find. You have temporal duplication when the same work is
done repeatedly when it doesn't have to be. This isn't a problem in
the same way as the previous two forms of duplication since it doesn't
involve having extra code laying about. It can become a
problem as the system scales and the repeated work starts taking too
much time. Various caching techques can be used to address this
problem.
Of the three types I've identified above, textual duplication is the
worst. It is the quickest to bite you as the system grows. It has a
nasty habit of being viral: when you see code duplicated in the
system, it can be tempted to think "Oh, well then, it's not a problem
if I just nic this code and put a copy of it over here and fiddle it a
bit. Hey, everyone else is doing it!" It's a lot like the Broken
Window that Dave Thomas & Andy Hunt talk about. If you don't keep on
top of the little things as they appear, the whole thing will
soon go down the toilet.
Summary
So, chances are pretty good that some of your code sucks.
What are you going to do about it? Well, first you have to
recognise the fact (and hopefully this article has helped with that)
and admit that there's a problem.
Then you have a choice. You can choose to do something about it.
You can learn new ways to work... ways that help you write better
code. That's great. If only one programmer out there does this, I'll
be happy.
Your other option is to do nothing about it, and continue writing
code the way you always have.
So this begs the question ... what makes code testable?
The best answer I found deals with complexity due to method paths and dependencies.
For method paths, any information on McCabe's cyclomatic complexity calculations is extremely helpful. Google is your friend here, as are programs such as JavaNCSS and devMetrics (C#).
Two good sources of information about dependencies are:
I don't know of any dependency analysis software available for .NET yet.
I believe that easy analysis of depencencies in an automated fashion depends on statically typed languages. Perhaps it can be done in a dynamically typed language, but it would be a much harder problem.
Are there any other factors that you know of that contribute to testable software?
I have seen a really good definition of testable code in Michael Feathers' blog:
I don't care how good you think your design is. If I can't walk in and write a test for an arbitrary method of yours in five minutes its not as good as you think it is, and whether you know it or not, you're paying a price for it.
It sounds like you're talking about the effects of testable code. You know when you're writing tests for it whether it's testable. And if you're writing tests at the same time as development, then you keep reworking the code until it is testable.
But what about going back and writing tests for code that already exists? What makes the class testable or not? What rules of thumb did programmer A apply that caused his/her classes to be more easily testable vs those that programmer B used that were not easily testable?
Basically, my question is this: What are the patterns and practices that facilitate testable code? That's the heart of the question I tried above (but apparently failed) to ask and then partially answer.
... > But what about going back and writing tests for code that > already exists? What makes the class testable or not? > What rules of thumb did programmer A apply that caused > d his/her classes to be more easily testable vs those that > programmer B used that were not easily testable?
The rule of thumb that programmer A used was probably to write the test(s) before writing the code.
But you're talking about pre-existing code...
> Basically, my question is this: What are the patterns and > practices that facilitate testable code? That's the heart > of the question I tried above (but apparently failed) to > ask and then partially answer.
Short answer is high cohesion and low coupling... which doesn't really say much. I would suggest that the most direct and effective way to learn what properties testable code should have would be to practice test-driven development (TDD) and observe the result. I'm not aware of any papers/articles that are specific and thorough about the design properties resulting from TDD.
I agree with most everything you have to say. However, while the DRY principle is almost universally applicable, like most principles, there are times when violation is acceptable. The most notable is when you want to manage dependencies. If avoiding duplication means creating a heavy dependency on another component, then one might consider copying the code in favor of creating the dependency.
Certainly, refactoring can help in this situation, but there are some situations, such as when the dependency would be on a 3rd party component, where refactoring isn't always an option.
Principles, patterns, heuristics, guidelines are all valuable, but as developers, we still have to think about when they're appropriate, and when they aren't. Unfortunately, I think we too often rely on principles, etc. to do the thinking for us.
Well, everyone seems to be commenting about the difficulties of testing, so I'll mention my article on unit test patterns, part V on a series on Advanced Unit Testing (also note that AUT is an open source project hosted on Tigris).
"Choose style and formating conventions early in the project, and conform to them. Require that everyone conform to them. This will be easier to do if everyone has a hand in deciding on the conventions and buys into them. This will make the code uniform and thus easier to read."
I'm still skeptical of this. Of course, I'm in favour of sensible naming and layout. And I think most experienced programmers already do this. But I don't believe this should automatically imply setting standards across a project. Particularly for indentation and line-length.
The problem with standardizing those is that they depend a lot on the screen you're using. Are you using a 15 inch laptop or a 21 inch desktop? Do you prefer your windows split vertically or horizontally? Demanding a convention there is tantamount to demanding conformity of the whole screen layout. I simply don't believe any benefits of standard line length outweigh the cost in this case.
Most code will suck at some point during its development and improving code is an iterative process, I like to state the process as:
1. Prove that it works. "Your code sucks if it doesn't work." "Your code sucks if it isn't testable."
2. Make it express itself clearly unless that conflicts with rule 1. "Your code sucks if it's hard to read." "Your code sucks if it's not understandable."
3. Remove duplication unless that conflicts with rule 2. "Your code sucks if it has duplication."
4. Make it efficient unless that conflicts with rule 3. "You have temporal duplication when the same work is done repeatedly when it doesn't have to be."
In practice, meaningful conflicts between the rules hardly ever happen getting 1 and 2 right mean that you often get 3 and 4 for free. I think that if you find yourself running into conflicts very often then your code really sucks and at that point it's worth getting a colleague to sense-check your early decisions.
Another nuance is that you can make your code suck less - it's not an either/or decision.
The difference between great programmers and good hackers is that in watching a great programmer code you get the sense that they can see the arc that code must take from working but sucking to it being great code. Hackers tend to stop when it works.
Okay, I admit that at least some of the code I have written in my life did not live up to the high standard I demand of myself. ;-)
I like much of the content of Dave's treatise, but there are a few items that bear further discussion.
A statement that bothers me is: >>>Your code sucks if it doesn't work. While that may sound like some obvious tenet on its face, I humbly submit that it is not true at least inasmuch as by using logic, the contrapositive is questionable. The contrapositive of the statement above is: >>>Your code works if it does not suck. That doesn't seem like it could really be true. Your code may be incorrect if it doesn't work, but it may actually be good (non-sucking) code if it follows several of the other rules outlined in the treatise. In particular, if your code is testable and readable, defects can be readily found, fixed, and verified. Moreover, if it is truly understandable (and that is a high standard), it may even be refactored as necessary.
Have you ever tried to refactor code that you didn't understand? What people really do when they try to refactor code that they don't understand is either a) break it or b) rewrite it or c) first break it and then rewrite it. Rewriting is not the same as refactoring. Rewriting tends to result in such problems as functional duplication. Refactoring does not.
Another problematic portion of the treatise, in my opinion, is found in the two competing desires: >>>Your code sucks if it's not understandable. >>>Your code sucks if it has duplication. I propose that if your code has sections that are similar, that is not necessarily undesirable duplication; it may actually be a desirable pattern-based design. It is true that in many such cases, a programmer can eliminate the duplication by applying one or more techniques (including those itemized in the treatise). However, it is not always desirable to do so. I warn all programmers that in many cases, applying such refactoring techniques increases the complexity of the static model, and increasing the complexity of the static model beyond certain limited bounds actually decreases understanding. For example, the deeper a type hierarchy gets (past 5 levels or so), the less most programmers can retain it. Or, the more functionality is generalized at an abstraction and specialized by the concrete, the less most programmers follow the logic and interactions.
I fully agree with all your comments, except for one: "Why not use public fields, instead of private fiels plus getters and setters". Your argument is correct that in many cases these getters and setters don't do anything extra. But the point is that a number of frameworks rely on the fact that there is a getter and setter, for example Hibernate BeanUtils, Struts etc etc. To create all these getters/setters is just two mouseclicks (in Exclipse at least) and Java will handle these getters as efficiently as public fields.
> But the point is that a number of frameworks rely on the > fact that there is a getter and setter, for example > Hibernate BeanUtils, Struts etc etc.
Yes. and in those cases you need to add getters/setters. but when all you want is a publically accessible variable... just make it public.. and, of course, question why you want a publically accessible variable in the first place.
Great article (and yes I have written code that sucks in many ways), although I will take issue with the idea of haveing publically accessible variables for two reasons.
1. Consistency and Readability. There will be, in almost all cases, some getter and setter methods that do more than simply set and return the variable. Rather than trying to remember wich variables are public and which are available through getter and setter methods, simply make them all getters and setters.
2. Maintainability. I have on more than one occation changed a getter and/or setter from simply passing the value to actually doing some formatting/parsing or value checking. If the variable is available publically I cannot do this without breaking other code.
> 1. Consistency and Readability. > There will be, in almost all cases, some getter and setter > methods that do more than simply set and return the > variable. Rather than trying to remember wich variables > are public and which are available through getter and > setter methods, simply make them all getters and setters.
I'd argue that you should try to not use getters and setters at all if you can avoid them. Many programmers (I'm tempted to say most.. based on my sampling) still haven't grokked the real idea behind encapsulation. This is especially true in the Java world where getters/setters are central parts of various paradigms & frameworks (e.g. JavaBeans). Go read some really good OO code. If you put the behaviour where it belongs you won't need nearly as many getters/setters.
> 2. Maintainability. > I have on more than one occation changed a getter and/or > setter from simply passing the value to actually doing > some formatting/parsing or value checking. If the > variable is available publically I cannot do this without > breaking other code.
I don't accept that as an excuse anymore.. with tools like Eclipse & IDEA. They can find references to the vars.. and often even do the switch to a getter/setter for you.
"an experienced OO designer could probably eliminate 99 percent of the accessors currently in your code without much difficulty"
and:
"Getter/setter methods often make their way in code because the coder was thinking procedurally. The best way to break out of that procedural mindset is to think in terms of a conversation between objects that have well-defined responsibilities."
What's needed is a solid understanding of OO. I'm constantly amazed that, after 37 years, that's still mostly absent!
Maybe I should change my mind regarding certification & licensing of programmers.
Dave
Flat View: This topic has 21 replies
on 2 pages
[
12
|
»
]
Previous Topic
