Skip to content

A useful C++ interview question

It’s surprising how few people who say they know C++ really know C++. This is my favourite basic C++ interview question.

Lets say you want to write your own, simple fixed-size vector class for integers. The specification is simple — construct the vector with one argument, the size of the vector. Construction creates a vector of the given size, all initialized to zero. Support accessors via indexing.

Here’s the first try (which I write on the whiteboard for the candidate):

#include "stdio.h"
#define OUT_OF_RANGE 1

class Vec {
  public:
    Vec(int sz) : sz_(sz) { 
      data_ = new int[sz_]; 
      for (int i=0; i != sz_; ++i) { data_[i] = 0; }
    }

    ~Vec() { delete data_; }

    int size() { return sz_; }

    int& operator[](int i) {
      if (i < sz_) return data_[i];
      else throw OUT_OF_RANGE;
    }

    int sz_;
    int* data_;
};

int main() {
  Vec v(10);
  int i;
  for (i = 0; i != 10; ++i) v[i] = i+1;

  // this prints "1 2 3 4 5 6 7 8 9 10"
  for (i = 0; i != 10; ++i) printf("%d ", v[i]);
}

The question is this. The code, with a basic test, compiles and works as you might expect. Is there anything wrong with this code?

If your candidate stares at this and says “looks OK to me”, it’s probably time to end the interview and send them home. The correct answer, of course, is “many, many things.”

No encapsulation. The members sz_ and data_ should be declared private, otherwise clients could corrupt the data, intentionally or otherwise.

The destructor won’t delete the allocated array. It should say delete [] data_;. If they don’t know this, they have no idea of C++’s memory model.

It’s a mistake to use an integral size, since integers can be negative. In the given code, if you give a negative integer parameter to the constructor, new will throw an exception, or even if it doesn’t the for loop will be infinite. If you call an accessor with a negative index (v[-2]), it’ll access illegal memory. The size parameter should ideally be std::size_t, or at least unsigned.

The compiler will auto-generate crash-inducing code for copy construction and assignment, since these haven’t been defined. For example, the auto-generated assignment operator will look like this:

Vec& operator= (const Vec& rhs) {
  this.sz_ = rhs.sz_;
  this.data_ = rhs.data_;
  return *this;
}

Note that this copies the pointer to the array and not the array. The simple act of adding Vec v2(10); v2 = v1; at the end of the main function above will cause delete to be called twice on the same pointer, when v1 and v2 are destructed, leading to potential disaster. The candidate should be able to code both functions correctly.

You can’t use a const Vec object or reference for read-only operations, which basically cripples the class. If you want to pass a Vec to a function that only reads it, func(Vec v) results in an expensive copy operation, and func(Vec& v) or func(Vec* v) doesn’t guarantee that the vector won’t be mutated. Whats needed is to mark the size() function as const and add a const accessor:

const int& operator[](std::size_t i) const { 
  /* same code as non-const */
}

An implicit constructor could lead to uncaught bugs at compile time. For example, a function with the signature myFunc(Vec v) could be called as myFunc(5) with an implicit Vec object being constructed as the argument. In almost all cases, this is unintentional and erroneous, and the constructor should be marked with the explicit keyword.

What I like about this question is that the quality of the candidate’s answers tell you a lot. Some candidates will surprise you and go beyond the basic issues listed above. If a candidate gets through these quickly, that’s a very good sign and you can go on to further improving the design, using templates, dynamic re-allocations etc.

2 Comments