C.con: Containers and other resource handles

A container is an object holding a sequence of objects of some type; std::vector is the archetypical container. A resource handle is a class that owns a resource; std::vector is the typical resource handle; its resource is its sequence of elements.

Summary of container rules:

See also: Resources

C.lambdas: Function objects and lambdas

A function object is an object supplying an overloaded () so that you can call it. A lambda expression (colloquially often shortened to "a lambda") is a notation for generating a function object. Function objects should be cheap to copy (and therefore passed by value).

Summary:

C.hier: Class hierarchies (OOP)

A class hierarchy is constructed to represent a set of hierarchically organized concepts (only). Typically base classes act as interfaces. There are two major uses for hierarchies, often named implementation inheritance and interface inheritance.

Class hierarchy rule summary:

Designing rules for classes in a hierarchy summary:

Accessing objects in a hierarchy rule summary:

C.120: Use class hierarchies to represent concepts with inherent hierarchical structure (only)

Reason

Direct representation of ideas in code eases comprehension and maintenance. Make sure the idea represented in the base class exactly matches all derived types and there is not a better way to express it than using the tight coupling of inheritance.

Do not use inheritance when simply having a data member will do. Usually this means that the derived type needs to override a base virtual function or needs access to a protected member.

Example
??? Good old Shape example?
Example, bad

Do not represent non-hierarchical domain concepts as class hierarchies.

template<typename T>
class Container {
public:
    // list operations:
    virtual T& get() = 0;
    virtual void put(T&) = 0;
    virtual void insert(Position) = 0;
    // ...
    // vector operations:
    virtual T& operator[](int) = 0;
    virtual void sort() = 0;
    // ...
    // tree operations:
    virtual void balance() = 0;
    // ...
};

Here most overriding classes cannot implement most of the functions required in the interface well. Thus the base class becomes an implementation burden. Furthermore, the user of Container cannot rely on the member functions actually performing a meaningful operations reasonably efficiently; it may throw an exception instead. Thus users have to resort to run-time checking and/or not using this (over)general interface in favor of a particular interface found by a run-time type inquiry (e.g., a dynamic_cast).

Enforcement
  • Look for classes with lots of members that do nothing but throw.
  • Flag every use of a nonpublic base class B where the derived class D does not override a virtual function or access a protected member in B, and B is not one of the following: empty, a template parameter or parameter pack of D, a class template specialized with D.

C.121: If a base class is used as an interface, make it a pure abstract class

Reason

A class is more stable (less brittle) if it does not contain data. Interfaces should normally be composed entirely of public pure virtual functions and a default/empty virtual destructor.

Example
class my_interface {
public:
    // ...only pure virtual functions here ...
    virtual ~my_interface() {}   // or =default
};
Example, bad
class Goof {
public:
    // ...only pure virtual functions here ...
    // no virtual destructor
};

class Derived : public Goof {
    string s;
    // ...
};

void use()
{
    unique_ptr<Goof> p {new Derived{"here we go"}};
    f(p.get()); // use Derived through the Goof interface
    g(p.get()); // use Derived through the Goof interface
 } // leak

The Derived is deleted through its Goof interface, so its string is leaked. Give Goof a virtual destructor and all is well.

Enforcement
  • Warn on any class that contains data members and also has an overridable (non-final) virtual function.

C.122: Use abstract classes as interfaces when complete separation of interface and implementation is needed

Reason

Such as on an ABI (link) boundary.

Example
struct Device {
    virtual void write(span<const char> outbuf) = 0;
    virtual void read(span<char> inbuf) = 0;
};

class D1 : public Device {
    // ... data ...

    void write(span<const char> outbuf) override;
    void read(span<char> inbuf) override;
};

class D2 : public Device {
    // ... differnt data ...

    void write(span<const char> outbuf) override;
    void read(span<char> inbuf) override;
};

A user can now use D1s and D2s interrchangeably through the interface provided by Device. Furthermore, we can update D1 and D2 in a ways that are not binarily compatible with older versions as long as all access goes through Device.

Enforcement
???

C.hierclass: Designing classes in a hierarchy:

C.126: An abstract class typically doesn't need a constructor

Reason

An abstract class typically does not have any data for a constructor to initialize.

Example
???
Exceptions
  • A base class constructor that does work, such as registering an object somewhere, may need a constructor.
  • In extremely rare cases, you might find it reasonable for an abstract class to have a bit of data shared by all derived classes (e.g., use statistics data, debug information, etc.); such classes tend to have constructors. But be warned: Such classes also tend to be prone to requiring virtual inheritance.
Enforcement

Flag abstract classes with constructors.

C.127: A class with a virtual function should have a virtual or protected destructor

Reason

A class with a virtual function is usually (and in general) used via a pointer to base. Usually, the last user has to call delete on a pointer to base, often via a smart pointer to base, so the destructor should be public and virtual. Less commonly, if deletion through a pointer to base is not intended to be supported, the destructor should be protected and nonvirtual; see C.35.

Example, bad
struct B {
    virtual int f() = 0;
    // ... no user-written destructor, defaults to public nonvirtual ...
};

struct D : B {   // bad: class with a resource derived from a class without a virtual destructor
    string s {"default"};
};

void use()
{
    auto p = make_unique<D>();
    // ...
} // calls B::~B only, leaks the string
Note

There are people who don't follow this rule because they plan to use a class only through a shared_ptr: std::shared_ptr<B> p = std::make_shared<D>(args); Here, the shared pointer will take care of deletion, so no leak will occur from an inappropriate delete of the base. People who do this consistently can get a false positive, but the rule is important -- what if one was allocated using make_unique? It's not safe unless the author of B ensures that it can never be misused, such as by making all constructors private and providing a factory function to enforce the allocation with make_shared.

Enforcement
  • A class with any virtual functions should have a destructor that is either public and virtual or else protected and nonvirtual.
  • Flag delete of a class with a virtual function but no virtual destructor.

C.128: Virtual functions should specify exactly one of virtual, override, or final

Reason

Readability. Detection of mistakes. Writing explicit virtual, override, or final is self-documenting and enables the compiler to catch mismatch of types and/or names between base and derived classes. However, writing more than one of these three is both redundant and a potential source of errors.

Use virtual only when declaring a new virtual function. Use override only when declaring an overrider. Use final only when declaring an final overrider.

Example, bad
struct B {
    void f1(int);
    virtual void f2(int) const;
    virtual void f3(int);
    // ...
};

struct D : B {
    void f1(int);      // warn: D::f1() hides B::f1()
    void f2(int) const;      // warn: no explicit override
    void f3(double);   // warn: D::f3() hides B::f3()
    // ...
};

struct D2 : B {
    virtual void f2(int) final;  // BAD; pitfall, D2::f does not override B::f 
};
Enforcement
  • Compare names in base and derived classes and flag uses of the same name that does not override.
  • Flag overrides with neither override nor final.
  • Flag function declarations that use more than one of virtual, override, and final.

C.129: When designing a class hierarchy, distinguish between implementation inheritance and interface inheritance

Reason

??? Herb: I've become a non-fan of implementation inheritance -- seems most often an anti-pattern. Are there reasonable examples of it?

Example
???
Enforcement

???

C.130: Redefine or prohibit copying for a base class; prefer a virtual clone function instead

Reason

Copying a base is usually slicing. If you really need copy semantics, copy deeply: Provide a virtual clone function that will copy the actual most-derived type and return an owning pointer to the new object, and then in derived classes return the derived type (use a covariant return type).

Example
class base {
public:
    virtual owner<base*> clone() = 0;
    virtual ~base() = 0;

    base(const base&) = delete;
    base& operator=(const base&) = delete;
};

class derived : public base {
public:
    owner<derived*> clone() override;
    virtual ~derived() override;
};

Note that because of language rules, the covariant return type cannot be a smart pointer. See also C.67.

Enforcement
  • Flag a class with a virtual function and a non-user-defined copy operation.
  • Flag an assignment of base class objects (objects of a class from which another has been derived).

C.131: Avoid trivial getters and setters

Reason

A trivial getter or setter adds no semantic value; the data item could just as well be public.

Example
class point {
    int x;
    int y;
public:
    point(int xx, int yy) : x{xx}, y{yy} { }
    int get_x() { return x; }
    void set_x(int xx) { x = xx; }
    int get_y() { return y; }
    void set_y(int yy) { y = yy; }
    // no behavioral member functions
};

Consider making such a class a struct -- that is, a behaviorless bunch of variables, all public data and no member functions.

struct point {
    int x = 0;
    int y = 0;
};
Note

A getter or a setter that converts from an internal type to an interface type is not trivial (it provides a form of information hiding).

Enforcement

Flag multiple get and set member functions that simply access a member without additional semantics.

C.132: Don't make a function virtual without reason

Reason

Redundant virtual increases run-time and object-code size. A virtual function can be overridden and is thus open to mistakes in a derived class. A virtual function ensures code replication in a templated hierarchy.

Example, bad
template<class T>
class Vector {
public:
    // ...
    virtual int size() const { return sz; }   // bad: what good could a derived class do?
private:
    T* elem;   // the elements
    int sz;    // number of elements
};

This kind of "vector" isn't meant to be used as a base class at all.

Enforcement
  • Flag a class with virtual functions but no derived classes.
  • Flag a class where all member functions are virtual and have implementations.

C.133: Avoid protected data

Reason

protected data is a source of complexity and errors. protected data complicated the statement of invariants. protected data inherently violates the guidance against putting data in base classes, which usually leads to having to deal virtual inheritance as well.

Example
???
Note

Protected member function can be just fine.

Enforcement

Flag classes with protected data.

C.134: Ensure all non-const data members have the same access level

Reason

Prevention of logical confusion leading to errors. If the non-const data members don't have the same access level, the type is confused about what it's trying to do. Is it a type that maintains an invariant or simply a collection of values?

Discussion

The core question is: What code is responsible for maintaining a meaningful/correct value for that variable?

There are exactly two kinds of data members:

  • A: Ones that don’t participate in the object’s invariant. Any combination of values for these members is valid.
  • B: Ones that do participate in the object’s invariant. Not every combination of values is meaningful (else there’d be no invariant). Therefore all code that has write access to these variables must know about the invariant, know the semantics, and know (and actively implement and enforce) the rules for keeping the values correct.

Data members in category A should just be public (or, more rarely, protected if you only want derived classes to see them). They don’t need encapsulation. All code in the system might as well see and manipulate them.

Data members in category B should be private or const. This is because encapsulation is important. To make them non-private and non-const would mean that the object can't control its own state: An unbounded amount of code beyond the class would need to know about the invariant and participate in maintaining it accurately -- if these data members were public, that would be all calling code that uses the object; if they were protected, it would be all the code in current and future derived classes. This leads to brittle and tightly coupled code that quickly becomes a nightmare to maintain. Any code that inadvertently sets the data members to an invalid or unexpected combination of values would corrupt the object and all subsequent uses of the object.

Most classes are either all A or all B:

  • All public: If you're writing an aggregate bundle-of-variables without an invariant across those variables, then all the variables should be public. By convention, declare such classes struct rather than class
  • All private: If you’re writing a type that maintains an invariant, then all the non-const variables should be private -- it should be encapsulated.
Exceptions

Occasionally classes will mix A and B, usually for debug reasons. An encapsulated object may contain something like non-const debug instrumentation that isn’t part of the invariant and so falls into category A -- it isn’t really part of the object’s value or meaningful observable state either. In that case, the A parts should be treated as A's (made public, or in rarer cases protected if they should be visible only to derived classes) and the B parts should still be treated like B’s (private or const).

Enforcement

Flag any class that has non-const data members with different access levels.

C.135: Use multiple inheritance to represent multiple distinct interfaces

Reason

Not all classes will necessarily support all interfaces, and not all callers will necessarily want to deal with all operations. Especially to break apart monolithic interfaces into "aspects" of behavior supported by a given derived class.

Example
???
Note

This is a very common use of inheritance because the need for multiple different interfaces to an implementation is common and such interfaces are often not easily or naturally organized into a single-rooted hierarchy.

Note

Such interfaces are typically abstract classes.

Enforcement

???

C.136: Use multiple inheritance to represent the union of implementation attributes

Reason

??? Herb: Here's the second mention of implementation inheritance. I'm very skeptical, even of single implementation inheritance, never mind multiple implementation inheritance which just seems frightening -- I don't think that even policy-based design really needs to inherit from the policy types. Am I missing some good examples, or could we consider discouraging this as an anti-pattern?

Example
???
Note

This a relatively rare use because implementation can often be organized into a single-rooted hierarchy.

Enforcement

??? Herb: How about opposite enforcement: Flag any type that inherits from more than one non-empty base class?

C.137: Use virtual bases to avoid overly general base classes

Reason

???

Example
???
Note

???

Enforcement

???

C.138: Create an overload set for a derived class and its bases with using

Reason

???

Example
???

C.139: Use final sparingly

Reason

Capping a hierarchy with final is rarely needed for logical reasons and can be damaging to the extensibility of a hierarchy. Capping an individual virtual function with final is error-prone as that final can easily be overlooked when defining/overriding a set of functions.

Example, bad
class Widget { /* ... */ };

class My_widget final : public Widget { /* ... */ };    // nobody will ever want to improve My_widget (or so you thought)

class My_improved_widget : public My_widget { /* ... */ };  // error: can't do that
Example, bad
struct Interface {
    virtual int f() = 0;
    virtual int g() = 0;
};

class My_implementation : public Interface {
    int f() override;
    int g() final;  // I want g() to be FAST!
    // ...
};

class Better_implementation : public My_implementation {
    int f();
    int g();
    // ...
};

void use(Interface* p)
{
    int x = p->f();    // Better_implementation::f()
    int y = p->g();    // My_implementation::g() Surprise?
}

// ...

use(new Better_interface{});

The problem is easy to see in a small example, but in a large hierarchy with many virtual functions, tools are required for reliably spotting such problems. Consistent use of override would catch this.

Note

Claims of performance improvements from final should be substantiated. Too often, such claims are based on conjecture or experience with other languages.

There are examples where final can be important for both logical and performance reasons. One example is a performance-critical AST hierarchy in a compiler or language analysis tool. New derived classes are not added every year and only by library implementers. However, misuses are (or at least has been) far more common.

Enforcement

Flag uses of final.

C.140: Do not provide different default arguments for a virtual function and an overrider

Reason

That can cause confusion: An overrider do not inherit default arguments..

Example, bad
class base {
public:
    virtual int multiply(int value, int factor = 2) = 0;
};

class derived : public base {
public:
    int multiply(int value, int factor = 10) override;
};

derived d;
base& b = d;

b.multiply(10);  // these two calls will call the same function but
d.multiply(10);  // with different arguments and so different results
Enforcement

Flag default arguments on virtual functions if they differ between base and derived declarations.

C.hier-access: Accessing objects in a hierarchy

C.145: Access polymorphic objects through pointers and references

Reason

If you have a class with a virtual function, you don't (in general) know which class provided the function to be used.

Example
struct B { int a; virtual int f(); };
struct D : B { int b; int f() override; };

void use(B b)
{
    D d;
    B b2 = d;   // slice
    B b3 = b;
}

void use2()
{
    D d;
    use(d);   // slice
}

Both ds are sliced.

Exception

You can safely access a named polymorphic object in the scope of its definition, just don't slice it.

void use3()
{
    D d;
    d.f();   // OK
}
Enforcement

Flag all slicing.

C.146: Use dynamic_cast where class hierarchy navigation is unavoidable

Reason

dynamic_cast is checked at run time.

Example
struct B {   // an interface
    virtual void f();
    virtual void g();
};

struct D : B {   // a wider interface
    void f() override;
    virtual void h();
};

void user(B* pb)
{
    if (D* pd = dynamic_cast<D*>(pb)) {
        // ... use D's interface ...
    }
    else {
        // ... make do with B's interface ...
    }
}
Note

Like other casts, dynamic_cast is overused. Prefer virtual functions to casting. Prefer static polymorphism to hierarchy navigation where it is possible (no run-time resolution necessary) and reasonably convenient.

Note

Some people use dynamic_cast where a typeid would have been more appropriate; dynamic_cast is a general "is kind of" operation for discovering the best interface to an object, whereas typeid is a "give me the exact type of this object" operation to discover the actual type of an object. The latter is an inherently simpler operation that ought to be faster. The latter (typeid) is easily hand-crafted if necessary (e.g., if working on a system where RTTI is - for some reason - prohibited), the former (dynamic_cast) is far harder to implement correctly in general.

Consider:

struct B {
    const char * name {"B"};
    virtual const char* id() const { return name; }
    // ...
};

struct D : B {
    const char * name {"D"};
    const char* id() const override { return name; }
    // ...
};

void use()
{
    B* pb1 = new B;
    B* pb2 = new D;

    cout << pb1->id(); // "B"
    cout << pb2->id(); // "D"

    if (pb1->id() == pb2->id()) // *pb1 is the same type as *pb2
    if (pb2 == "D") {         // looks innocent
            D* pd = static_cast<D*>(pb1);
            // ...
    }
    // ...
}

The result of pb2 == "D" is actually implementation defined. We added it to warn of the dangers of home-brew RTTI. This code may work as expected for years, just to fail on a new machine, new compiler, or a new linker that does not unify character literals.

If you implement your own RTTI, be careful.

Exceptions

If your implementation provided a really slow dynamic_cast, you may have to use a workaround. However, all workarounds that cannot be statically resolved involve explicit casting (typically static_cast) and are error-prone. You will basically be crafting your own special-purpose dynamic_cast. So, first make sure that your dynamic_cast really is as slow as you think it is (there are a fair number of unsupported rumors about) and that your use of dynamic_cast is really performance critical.

We are of the opinion that current implementations of dynamic_cast are unnecessarily slow. For example, under suitable conditions, it is possible to perform a dynamic_cast in fast constant time. However, compatibility makes changes difficult even if all agree that an effort to optimize is worthwhile.

In very rare cases, if you have measured that the dynamic_cast overhead is material, you have other means to statically guarantee that a downcast will succeed (e.g., you are using CRTP carefully), and there is no virtual inheritance involved, consider tactically resorting static_cast with a prominent comment and disclaimer summarizing this paragraph and that human attention is needed under maintenance because the type system can't verify correctness. Even so, in our experience such "I know what I'm doing" situations are still a known bug source.

Enforcement

Flag all uses of static_cast for downcasts, including C-style casts that perform a static_cast.

C.147: Use dynamic_cast to a reference type when failure to find the required class is considered an error

Reason

Casting to a reference expresses that you intend to end up with a valid object, so the cast must succeed. dynamic_cast will then throw if it does not succeed.

Example
???
Enforcement

???

C.148: Use dynamic_cast to a pointer type when failure to find the required class is considered a valid alternative

Reason

???

Example
???
Enforcement

???

C.149: Use unique_ptr or shared_ptr to avoid forgetting to delete objects created using new

Reason

Avoid resource leaks.

Example
void use(int i)
{
    auto p = new int {7};           // bad: initialize local pointers with new
    auto q = make_unique<int>(9);   // ok: guarantee the release of the memory allocated for 9
    if (0 < i) return;              // maybe return and leak
    delete p;                       // too late
}
Enforcement
  • Flag initialization of a naked pointer with the result of a new
  • Flag delete of local variable

C.150: Use make_unique() to construct objects owned by unique_ptrs

Reason

make_unique gives a more concise statement of the construction. It also ensures exception safety in complex expressions.

Example
unique_ptr<Foo> p {new<Foo>{7}};   // OK: but repetitive

auto q = make_unique<Foo>(7);      // Better: no repetition of Foo

// Not exception-safe: the compiler may interleave the computations of arguments as follows:
//
// 1. allocate memory for Foo,
// 2. construct Foo,
// 3. call bar,
// 4. construct unique_ptr<Foo>.
//
// If bar throws, Foo will not be destroyed, and the memory allocated for it will leak.
f(unique_ptr<Foo>(new Foo()), bar());

// Exception-safe: calls to functions are never interleaved.
f(make_unique<Foo>(), bar());
Enforcement
  • Flag the repetitive usage of template specialization list <Foo>
  • Flag variables declared to be unique_ptr<Foo>

C.151: Use make_shared() to construct objects owned by shared_ptrs

Reason

make_shared gives a more concise statement of the construction. It also gives an opportunity to eliminate a separate allocation for the reference counts, by placing the shared_ptr's use counts next to its object.

Example
shared_ptr<Foo> p {new<Foo>{7}};   // OK: but repetitive; and separate allocations for the Foo and shared_ptr's use count

auto q = make_shared<Foo>(7);   // Better: no repetition of Foo; one object
Enforcement
  • Flag the repetitive usage of template specialization list<Foo>
  • Flag variables declared to be shared_ptr<Foo>

C.152: Never assign a pointer to an array of derived class objects to a pointer to its base

Reason

Subscripting the resulting base pointer will lead to invalid object access and probably to memory corruption.

Example
struct B { int x; };
struct D : B { int y; };

void use(B*);

D a[] = {{1, 2}, {3, 4}, {5, 6}};
B* p = a;     // bad: a decays to &a[0] which is converted to a B*
p[1].x = 7;   // overwrite D[0].y

use(a);       // bad: a decays to &a[0] which is converted to a B*
Enforcement
  • Flag all combinations of array decay and base to derived conversions.
  • Pass an array as a span rather than as a pointer, and don't let the array name suffer a derived-to-base conversion before getting into the span