Where leading programmers explain how they find
unusual and carefully designed solutions

Michael Feathers

Michael Feathers

Recent Posts

Michael Feathers

Resource management is one of the more painful things that we have to deal with in software. Invariably, when we acquire something we have to release it. Garbage collected languages pull this burden away from us with respect to memory management, but if we have GC we still have to manually manage open file handles, database connections, and caches.

Recently, I got involved in a discussion of resource management with some C++ programmers, and I was struck by their attachment to RAII, a resource management idiom that is pervasive in C++ code. Some of them were of the opinion that RAII is an essential language feature. That's fair enough. I program in C++ and I'm attached to many of its features, but the thing that shocked me was the fact that I think RAII is a neat trick, but one with some significant downsides compared to other possible language features.

So, what is RAII and what are its downsides?

RAII stands for Resource Acquisition Is Initialization. It’s a technique which leverages the semantics of stack-based objects to tackle the resource management problem. Here’s a very simple example of RAII with error checking: removed for clarity:

class writeablefile {
    writeablefile(char *filename) : fp(fopen(filename, “w”) {}
    ~writablefile() { fclose(fp); }

    void write(char *text) { fprintf(fp, "%s", text); }
private:
    FILE *fp;
};
To use a writeable file, you create it on the stack like this:
   writeablefile outfile(“out.txt”);
And call write:
    outfile.write(“Hello there..\n”);

The thing that makes this nice is that the file is closed automatically when the outfile object goes out of scope: you don’t have to call a close function.

In C++, we use RAII for all sorts of things. We use it for file I/O, locks, smart pointers, just about any resource, and it’s a good fit, but it’s definitely not the only way to approach resource management. Let’s look at an alternative. Here’s some Ruby code that performs the same operation:

File.open("out.txt", "w+") do |outfile|
   outfile.write("hello there..\n")
end

In this code, the function open receives a block (all of the code between do and end). It then opens a file, passes a file object to the block, executes the block, and then closes the file.

This pattern is often known as Execute Around Method (EAM), and it’s a great way of approaching resource management problems. You put acquisition and release in one method and delegate to another which does the guts of what you need to do.

RAII and EAM.. Is there any reason to prefer one to the other? When I work in C++, I use RAII because it is idiomatic. There are a few places where EAM shows up naturally but they are rare. EAM is painful in languages which don’t have blocks, closures, or anonymous functions. In those languages you have to create new classes or objects to pass along. Both C++ and Java are getting closures in their next editions, so there is a chance that EAM will become more popular and I hope it does.

RAII is elegant, but I think that it has two warts. One is lack of explicitness. RAII makes resource acquisition a side effect of object creation. When you want to take advantage of it, you are often left struggling for a good class name that will make the RAII-ness of an object clear. Here’s an example. People often write RAII objects to manage semaphores and mutexes. You can create a lock on the stack, it acquires a mutex and then releases it when it goes out of scope i.e., when the function exits:

void some_function() {
  Lock lock(&mutex);

  ...
}

It’s fine. It works, but it is hard to argue that it is as clear as:

void some_function() {
  Lock lock(&mutex);
  lock.acquire();

  ...
  lock.release();
}

Does the name Lock really tell us that we are declaring something that locks a scope automatically? The boost libraries have an abstraction that does this called lock_guard. The name is a bit better, but it's a shame that it came so late. There are many RAII classes with names that don't adequately convey the semantics.

Cases where an abstraction supplies a manual acquisition protocol often let do more questions:

void some_function() {
  Lock lock(&mutex);
  lock.acquire();

  ...
  // no release, is the destructor handling it or is it a bug?
}

In code where RAII is used sporadically, you often have to hunt to figure out whether you are using an RAII enabled abstraction.

The second wart is way that RAII affects scope and maintenance. This is a more subtle point. When you use EAM, you create a new scope. You create a block that will execute inside the EAM, and, chances are, it will have a distinct responsibility. Yes, you can mix concerns by nesting EAMs like this:

File.open("out.txt", "w+") do |outfile|
   outfile.write("hello there..\n")
   ...
   File.open("out2.txt", "w+") do |outfile2|
       outfile2.write("hello other file..\n")
   end
   ...
end

But luckily, that sort of thing bothers most people. The RAII case, on the other hand, doesn’t seem to bother as many people:

void some_function()
{
  writeablefile outfile(“out.txt”);
  writeablefile outfile2(“out2.txt”);

  // mixed code to write to both files
  ...
}

It’s nice when a language feature makes it more natural to focus on one thing at a time.

RAII definitely has advantages in C++. One of them is exception safety. If you throw an exception in a function that uses RAII to manage a lock, the destructor will be called. To do get the same behavior in a language like Java or Ruby, you have to use a try-finally or ensure block in an EAM method.

The fact that C++ doesn’t have a finally mechanism for exceptions makes RAII the tool of choice. I can’t help thinking, however, that as useful as RAII is, it was a pun built upon the semantics of stack-based objects in C++, and that EAM is a better general purpose mechanism.

Michael Feathers

I was visiting a team a while ago and we came across an area of code with extremely long methods. We looked around and tried to understand the code but its shape and intent was lost in this jungle of run-on statements.

The remedy for this sort of thing is usually very straightforward - you refactor - you select cohesive bits of code and extract them as methods. I was ready to do this, but one of the developers stopped me. "Wait!", he said, "You can't extract that, look at the logging."

I looked and noticed that all of the logging statements were prefaced with the names of the methods they were called from.

I said "Well, we can decide on an alternative or we can preface the calls with the names of the methods we extract." And, they said "No, we count on those names to maintain context. If we extract a method and then call it from someplace else we won't be able to debug our production problems."

I said "You can build nested scopes into your logging."

They said they'd mull it over. We looked at different code.

It was a brief episode, but it reminded me of how much I hate logging. Yes, I realize that it is necessary in many environments, but for me it is definitely in the "necessary evil" category.

Whenever possible, I like to restrict logging to particular level in a design. When it runs rampant through code, it makes the code harder to understand, and people are reluctant to change it.

Yes, logging is useful, but it is a very real form of coupling. If you log you have to manage the coupling it induces. If you don't, it will manage you.

Michael Feathers

I bet I know where your ugliest code is. I can usually find it when I visit a team. The first thing that I do is ask for an explanation of the architecture. I listen to the names and remember the key abstractions. Then, I pick one and ask about it. Does it have subclasses? Does it have plug-in points? I ask to see them, and that’s usually where I find the ugly code. It lives next to and below the architectural abstractions. It’s the “implementation code”, the stuff people write just to get something done.

Once, I visited a team in an organization with an SOA strategy. They had some very nasty code, but it looked good on paper. At the beginning of the project, they learned the pattern: design is something you with five or six classes that you arranged in an "SOA pattern." Once you did, you had a design and it was time to code. Any one of those classes became ten to twenty thousand lines of code. The developers knew it was okay because, after all, they had a design. And, somehow, the fact that they had a design made all of the cruft excusable. They had the clean bit and all of the other stuff just wasn't important.

Some of this, I think, is understandable. If you tell people that something is important, often they hear it as: everything else is unimportant. People just seem to be wired that way, but I think we're really remiss in the industry. We don't talk enough about this boundary.. the place where design hits "where you get the work done." It's a messy place, but it's a fertile place. It's where code for new requirements hangs out, and if you hunt, you can often find seeds of new abstractions there.

Design is never over and it isn't just a part of your system. It's something you have to do every day that you work on a system. If you're continually adding code there's no way you can expect that the first five abstractions you have in your software will be the only ones. They may not even be the key abstractions after a few months. You can ignore the problem and let more and more code fall outside the design boundary, but the right thing is to push the design boundary forward, to embrace all of the new code and assimilate it into your design. It will change your design. You won't be able to think about it in the same way, but that's fine. It's far better than working in the junk outside and wishing you were inside. And, tell you what.. there are a lot of people who are in that situation.

Michael Feathers

Lambdas and closures have just been voted into C++ 0x. Since this is a blog about beautiful code, I invite you to reflect on the relative beauty of a couple of code examples that were part of the lambda proposal.

Before lambda functions:


  class between {
    double low, high;
  public:
    between(double l, double u) : low(l), high(u) {}
    bool operator()(const employee& e) {
      return e.salary() >= low && e.salary() < high;
    }
  };




  double min_salary;
  ...
  std::find_if(employees.begin(), employees.end(),
                     between(min_salary, 1.1 * min_salary));

And now with a lambda function:

  double min_salary = ....
  ...
  double u_limit = 1.1 * min_salary;
  std::find_if(employees.begin(), employees.end(),
      [&](const employee& e) 
          { return e.salary() >= min_salary && e.salary() < u_limit;});

I suspect I'm not alone in liking the first find_if call better.

Here's the problem. When you use lambda functions, in any language, there is a tension between clutter and expressiveness. And, in fact, the person who wrote that example seems to have been aware of the tension. He introduced an explaining temporary variable u_limit in the lambda version, just to keep the clutter down.

This tension between clutter and expressiveness is unavoidable. The other day, I went back and forth between doing this in Haskell:

  signature :: String -> String
  signature = filter (\x -> elem x "{;}")  

and this:

  isSigChar :: Char -> Bool
  isSigChar = flip elem "{;}"

  signature :: String -> String
  signature filter isSigChar

I ended up choosing the first version - the lambda function version. But, Haskell helped me in that choice. It was a win because Haskell's syntax is relatively uncluttered.

Compare this again to the C++ example:

  std::find_if(employees.begin(), employees.end(),
      [&](const employee& e) 
          { return e.salary() >= min_salary && e.salary() < u_limit;});

The lambda in this example has a lot of noise. It has type declarations, and nearly more punctuation than text. The native clutter in C++ shifts the bar in the clutter versus expressiveness tradeoff.

I hope that C++ 0x programmers in exactly this situation will opt for the function object approach, but I suspect that they won't. Function objects are more painful to write. I suspect that, in the end, we'll see gnarlier calls peppering our C++.

Sadly, we've been on the slippery slope for a while already. I understand why, for instance, STL algorithms accept two iterators but it seems odd that there are no versions which enable simple forward iteration over a single container - just pass the container and go. Some teams write little adapter templates, but in most of the industry we have code all over the place which starts like this: find_if(employees.begin(), employees.end(), ...

Calls should be clean.

No, I have no doubt that people will find decent ways to use lambdas in C++, but they are going to have to stretch. This particular example highlights an important issue. The default clutter of a syntax matters. In another language, I'd probably use a lambda but in C++ I prefer this call:

  std::find_if(employees.begin(), employees.end(),
                    between(min_salary, 1.1 * min_salary));

It just reads better.

Michael Feathers

I know your code is copyrighted.

I'm not an expert in US Copyright law, but I do know that you don't have to register a copyright to have one. All you have to do is make some utterance on your behalf or someone else's and it's owned. Write code in a file - it's owned.

Why, then, do we have copyright notices in file headers? If you want evidence of plagiarism or illicit copying, you can diff files. Court cases have turned on that sort of evidence. No, it seems that a copyright notice in a file is a warning. And, at the very least, it does indicate an owner - it tells you whose file it is. But.. why do the notices have to be so long?

Source file headers bother me, not in principle, but because people rarely know when to stop. First you have the copyright notice, then you have the disclaimers of liability, and if you are really unfortunate, you have 50 to 1000 lines of version control comments. You know, that's what we have version control systems for - to keep all of that history. There isn't much value in keeping it in the code. It's not like a file is going to wander down to the corner on its own. Code lives in an eco-system. I dare anyone to take a random file from a "enterprise" project and attempt to use it independently. It just doesn't work like that.

For better or worse, files are seen as the smallest independently distributable units in software. I suppose that if you are a lawyer, you take notice of that fact and make the file the unit of diligence. You want the notice there so that it is plain and in the face of the the potential abuser. And, if a legal notice is small, that's fine. One line, two lines.. I can handle that. The thing that I want an alternative to is the mandatory page down on open that is part and parcel of most source code. It's a tax. It is (to draw an analogy) as if every car manufacturer put a reverse etched liability disclaimer in the windshields of cars - something that you as a driver can't avoid every time you sit down in the car.

Do we really need that? I don't think so, but I do know that in most organizations, no one wants to be the one who trimmed back the mandatory header. It's just too easy to avoid that decision.. it's too easy to not be he person who said "You know, I think we can make do with a single line copyright statement and a url to our legal notices." It's such a small inconsequential issue.. unless you're a programmer.

If you're a programmer you scroll, and scroll, and scroll.

Note: this blog was inspired by a hard-edged code review done by a few years ago at a client site by "Uncle Bob" Martin.