Justify your use of “friend” in C++

The use of friend in C++ is usually a sign of bad design where your tech leader may ask you to justify its use. There is no double that friend does have its usefulness in some cases. For example, when you want grant access of a class’s private members only to a specific external class. To most’s surprise, it enhances encapsulation. If a public member function is used instead, all clients will have access to it, including those you don’t want them to.

Here is a typical case. We want all clients to be able to get the data, but only tDataCreator to create/set it.

class tData
{
   friend class tDataCreator;
public:
   tData(): m_data(-1)
   {
   }
   int get()
   {
      return m_data;
   }
private:
   void set(int data)
   {
      m_data = data;
   }
   int m_data;
};

class tDataCreator;

But friend doesn’t come without price. In this case, if there are more private members, we cannot control the members the friend is allowed to access, without some other resorts.

Today when I was reviewing some code by a programmer in my team, I saw the use of friend. Frankly, I cannot remember the last time I saw it. A voice in my mind told me that something may get wrong. Below is his code.

Backgrounds

It’s a highly simplfied version of the actual code, to get to the point. Here is a brief backgrounds. Link Layer Discovery Protocol (LLDP) is a layer 2 protocol defined by IEEE as 802.1AB. It’s used by switches and end stations to discover their neighbors by exchanging LLDP frames called LLDPDU. The information of a device is stored in its local MIB database. The information of neighbors is kept in remote MIB database. You can use management protocols such as SNMP to pull those information back. That’s it! As you may imagine, a simple depth first search (DFS) suffices.

To further keep things simple, we assume no physical loops in the network. And information in a device’s remote MIB don’t form a loop. In reality, this is not true. A two-device’s ecosystem with one physical link in-between also has a “virtual” loop since each of them appears in the other’s remote MIB.

class tTopoNode
{
   friend class tTopology;

public:
   tTopoNode(const string &IP): m_IP(IP)
   {
   }
private:
   // This is why tTopology is made a friend of us.
   void addNeighbor(std::shared_ptr<tTopoNode> node)
   {
      m_neighbors.push_back(node);
   }

   typedef std::vector<std::shared_ptr<tTopoNode> > tNeighbors;
   string m_IP;
   tNeighbors m_neighbors;
};

class tTopology
{
public:
   shared_ptr<tTopoNode> buildTopology(const string &IP);
};

// Simplified version of LLDP remote database -- a list of neighbor IPs.
typedef std::vector<std::string> tLLDPRemoteMIB;
// External function. Access the device and read back its LLDP remote database.
tLLDPRemoteMIB readLLDPRemoteMIB(const std::string &IP);

// Assume no loop in physical topology as well as LLDP remote MIB.
// Error handling is also omitted.
shared_ptr<tTopoNode> tTopology::buildTopology(const string &IP)
{
   shared_ptr<tTopoNode> node(new tTopoNode(IP));
   tLLDPRemoteMIB remote = readLLDPRemoteMIB(IP);
   for (tLLDPRemoteMIB::const_iterator it = remote.begin(); it != remote.end(); ++it)
   {
      shared_ptr<tTopoNode> neighbor = buildTopology(*it);
      node->addNeighbor(neighbor);
   }
   return node;
}

A Second Thought

Cool! Looks like a reasonable one, no? It’s a valid one until I gave it a second thought: why do we need this friend at the first place?

We need it because tTopology is the only one allowed to add a neighbor to a tTopoNode. After that, tTopoNode is read-only to others. But why do we need addNeighbor()? Can’t we set all neighbors upon construction? This can be achieved if all its neighbors have already been created before the current node is created. Does that ring a bell to you? 😉

A Post-traversal Implementation

Yes! A post traversal is exactly what we need. If we finish all neighbors first before creation of the current node, problem is solved. Let’s give it a shot. Below is the changed part.

class tTopoNode;
typedef std::vector<std::shared_ptr<tTopoNode> > tNeighbors;

class tTopoNode
{
public:
   // Neighbors are now added in ctor so there is not separate setter anymore.
   tTopoNode(const string &IP, const tNeighbors &neighbors):
   m_IP(IP), m_neighbors(neighbors)
   {
   }
private:
   string m_IP;
   tNeighbors m_neighbors;
};

shared_ptr<tTopoNode> tTopology::buildTopology(const string &IP)
{
   tLLDPRemoteMIB remote = readLLDPRemoteMIB(IP);
   tNeighbors neighbors;
   for (tLLDPRemoteMIB::const_iterator it = remote.begin(); it != remote.end(); ++it)
   {
      neighbors.push_back(buildTopology(*it));
   }
   return shared_ptr<tTopoNode>(new tTopoNode(IP, neighbors));
}

This code does not run in a time-critical context so it’s OK to have an extra copy per neighbor. The increase in time is unobservable. In return, we get better readability (because of less code) and greater encapsulation!

Cracking the Oyster

I still remember what Jon Bentley says in chapter “Cracking the Oyster” of his book <<Programming Pearls>> , “My mistake was to answer his question..Defining the problem was about ninety percent of this battle”. It’s pretty much the same here. If the programmer could dive into the root cause, he would be well on the right way.