GotW #3

Home Blog Talks Books & Articles Training & Consulting

On the
blog		 RSS feed November 4: Other Concurrency Sessions at PDC
November 3: PDC'09: Tutorial & Panel		 October 26: Hoare on Testing
October 23: Deprecating export Considered for ISO C++0x

This is the original GotW problem and solution substantially as posted to Usenet. See the book Exceptional C++ (Addison-Wesley, 2000) for the most current solutions to GotW issues #1-30. The solutions in the book have been revised and expanded since their initial appearance in GotW. The book versions also incorporate corrections, new material, and conformance to the final ANSI/ISO C++ standard.

Using the Standard Library (or, Temporaries Revisited)
Difficulty: 3 / 10

You're much better off using standard library algorithms than handcrafting your own. Here we reconsider the example in the last GotW to demonstrate how many of the problems would have been avoided by simply reusing what's already available in the standard library.

Problem

How many of the pitfalls in GotW #2 could have been avoided in the first place if the programmer had just used a standard-library algorithm instead of handcrafting his own loop? (Note: as before, don't change the semantics of the function even though they could be improved.)

Recap

Original flawed version:

  string FindAddr( list<Employee> l, string name )
  {
    for( list<Employee>::iterator i = l.begin();
         i != l.end();
         i++ )
    {
      if( *i == name )
      {
        return (*i).addr;
      }
    }
    return "";
  }

Mostly fixed version, l.end() still being called redundantly:

  string FindAddr( const list<Employee>& l,
                   const string& name )
  {
    string addr;
    for( list<Employee>::const_iterator i = l.begin();
         i != l.end();
         ++i )
    {
      if( (*i).name == name )
      {
        addr = (*i).addr;
        break;
      }
    }
    return addr;
  }

Solution

With no other changes, simply using find() would have avoided two temporaries and almost all of the l.end() inefficiency in the original:

  string FindAddr( list<Employee> l, string name )
  {
    list<Employee>::iterator i =
      find( l.begin(), l.end(), name );

    if( i != l.end() )
    {
      return (*i).addr;
    }
    return "";
  }

Combining this with the other fixes:

  string FindAddr( const list<Employee>& l,
                   const string& name )
  {
    string addr;
    list<Employee>::const_iterator i =
      find( l.begin(), l.end(), name );

    if( i != l.end() )
    {
      addr = (*i).addr;
    }
    return addr;
  }

[Guideline] Reuse standard library algorithms instead of handcrafting your own. It's faster, easier, AND safer!

 

Copyright © 2009 Herb Sutter