Posts by Thaddaeus Frogley 
  1. From the Codeface, a Bug Fixing Story ( Counting comments... )
  2. Type Names Matter ( Counting comments... )
  3. Overflowing your Comfort Zone ( Counting comments... )
  4. Writing Code To Write Code ( Counting comments... )
  5. From the Codeface ( Counting comments... )
  6. The Making Of Pirate Pennywise ( Counting comments... )
  7. Introducing Flexible Array Members ( Counting comments... )
  8. Syd Sym Post-mortem ( Counting comments... )
  9. Three Tools I Love (And So Should You) ( Counting comments... )
  10. Water Off A Duck's Back ( Counting comments... )
  11. So You Wanna Be a Senior? ( Counting comments... )
  12. Check Yourself ( Counting comments... )
Technology/ Code /

Last week I was asked the following question. This is not unusual, but it being a real live game programming question, I thought perhaps, just perhaps, the answer might be of interest to more than just the person who asked it. Names have been changed to protect the innocent.

How do I stop this code generating a:

MyClass(123) : warning: 'result' may be used uninitialized in this function

Without pragma'ing the warning off ?



T Remove(int key)
{
  // !!! We need a pragma here or something to tell the compiler to ignore the
  // fact that "result" is not initialized.

  T result;

  m_Mutex.Lock();

  Iterator it = m_Map.Find(key);
  if (it != m_Map.End())
  {
    result = *it;
    m_Map.Erase(it);
  }

  m_Mutex.Unlock();

  return result;
}

To which I answered:

Burn it with fire.

Then, more seriously: As the code stands you can't fix this. It is a fundamental error in the interface design. Presumably this is a member of a container class template that deals with value types.

In my opinion the method in question should be changed to one or other of:

// returns true on success, contents of result will be set to the value
// returns false if key does not exist, the contents of result
bool Remove(u32 key, T* result);

Or

// where, if key does not exist, return value "default"
T Remove(u32 key, const T& default);

Or if Remove is widely used and changing the signature is too big a change, then consider have the "default" value specified during construction of the object.

Of the three possible solutions, I prefer the first.

Note the use of mutexes implies this is used in a multithreaded context, so double-checking "exists" before "remove" is unreliable (as well as inefficient), and so there is no way to check if the Remove operation was successful when using this container with built in types (int, float, etc) with the current interface methods.