Have I misapplied inheritance?

Question

I am trying to set up a program that can generate balance sheets based on summing a number of transactions, and present the results in a format like this:

The important attributes here are that the top-level account (e.g. Assets) is decomposed into a tree of sub-accounts, and only the lowest-level accounts ("leaves") track their own balances (the balances of higher-level accounts are just the sums of the balances of their sub-accounts).

My go-to approach is to use inheritance:

class Account{
   string name;
   virtual int getBalance() =0; //generic base class has no implementation
   virtual void addToBalance(int amount) =0;
};
class ParentAccount : public Account{
   vector<Account*> children;
   virtual int getBalance() {
      int result = 0;
      for (int i = 0; i < children.size(); i++)
          result += children[i]->getBalance();
      return result;
   }
   virtual void addToBalance(int amount) {
      cout << "Error: Cannot modify balance of a parent account" << endl;
      exit(1);
   }
};
class ChildAccount : public Account{
   int balance;
   virtual int getBalance() { return balance; }
   virtual void addToBalance(int amount) {balance += amount;}
};

The idea is that which accounts are present is not known at compile time, so the tree account must be generated dynamically. Inheritance is helpful here because it makes it easy to generate an arbitrarily deep tree structure (ParentAccounts can have children which are ParentAccounts), and because it makes it easy to implement functions like getBalance() using recursion.

Things get a bit awkward when I attempt to incorporate the features which are unique to the derived classes, such as modifying the balance (which should only be possible for ChildAccount objects, as ParentAccount balances are just defined by the balances of their children). My plan is that a function like processTransaction(string accountName, int amount) would search through the tree structure looking for an account with the correct name, then call addToBalance(amount) on that account (*note below). Since the tree structure above would only allow me to find an Account*, it would either be necessary to implement addToBalance(amount) for all classes, as I did above, or to dynamic_cast the Account* to a ChildAccount* before calling addToBalance(). The first option seems slightly more elegant, but the fact that it requires me to define ParentAccount::addToBalance() (albeit as an error) seems kind of weird to me.

My question is: Is there a name for this awkwardness, and a standard approach for resolving it, or am I just totally misapplying inheritance?

*Note: I recognize there is probably a more efficient way of organizing the accounts for searching, but my primary objective is to create a program which is intuitive to interpret and debug. Based on my current level of understanding, this comes at the price of computational efficiency (in this case at least).

Answer 1

Yes, you've guessed right that it's not a correct inheritance case.

virtual void addToBalance(int amount) {
   cout << "Error: Cannot modify balance of a parent account" << endl;
   exit(1);
}

clearly indicates that class ParentAccount : public Account is wrong: ParentAccount has no IS-A relationship with Account.

There are two ways to fix it: one is to disinherit ParentAccount. But getBalance() consistency shows that it could be overreaction. So you can just exclude addToBalance() from Account (and ParentAccount), and hierarchy would be correct.

Of course, that would mean that you'll have to obtain the ChildAccount pointer before calling addToBalance(), but you've got to do it anyway. Practical solutions are numerous, e.g. you could simply have two vectors in ParentAccount, one for the other ParentAccounts, another for ChildAccounts, or use dynamic_cast, or... (depends on what else you've got to do with the accounts).

The names of this awkwardness are breaking LSP (Liskov substitution principle), or, more simple, breaking IS-A relationship.

Answer 2

So you have a tree whose nodes are of two different types deriving from the same base, and you want to perform an operation on one type but not the other ... this sounds like a job for the visitor pattern. :)

The idea behind the visitor^ pattern is this: it provides a way for elements of complex structures (trees, graphs) to be operated on differently according to their type (which is known only at runtime), and where the specific operation itself may also be known only at runtime, without having to change the whole hierarchy the elements belong to (i.e. avoiding things like the "error" function implementation of addToBalance that you thought of). (^It has little to do with visiting, so it's probably misnamed - it is more of a way of achieving double dispatch for languages that don't natively support it.)

So you can have a collection of operations to perform on the elements, and the operations could be e.g. overloaded based on the type of the element. An easy way to do this is to define a base class for all operations (I call this the Visitor class below). The only thing it will contain are empty functions - one for each type of element an operation could potentially be performed on. These functions will be overridden by specific operations.

class Visitor {

  virtual void Visit(ParentAccount*) { /* do nothing by default*/ }
  virtual void Visit(ChildAccount*) { /* do nothing by default */ }
};

Now we create a specific class to perform AddToBalance on ChildAccounts only.

class AddToBalance : public Visitor {

  public:
  AddBalance(string _nameOfTarget, int _balanceToAdd) :
    nameOfTarget(_nameOfTarget), balanceToAdd(_balanceToAdd) {}

  void Visit(ChildAccount* _child) { //overrides Visit only for ChildAccount nodes
    if(child->name == _name)
      child->addToBalance(_balance); //calls a function SPECIFIC TO THE CHILD
  }

  private:
    string nameOfTarget;
    int _balanceToAdd;
};

Some changes to your original Account class.

class Account{
   vector<Account*> children; //assume ALL Account objects could have children; \
                              //for leaf nodes (ChildAccount), this vector will be empty
   string name;
   virtual int getBalance() =0; //generic base class has no implementation

   //no addToBalance function!

   virtual void Accept(Visitor* _visitor) {
     _visitor->Visit(this);
   }
};

Notice the Accept() function in the Account class, which simply takes a Visitor* as an argument and calls that visitor's Visit function on this. This is where the magic happens. At this point, the type of this as well as the type of the _visitor will be resolved. If this is of type ChildAccount and _visitor is of type AddToBalance, then the Visit function that will be called in _visitor->Visit(this); will be void AddToBalance::Visit(ChildAccount* _child).

Which just so happens to call _child->addToBalance(...); :

class ChildAccount : public Account{
   int balance;
   virtual int getBalance() { return balance; }
   virtual void addToBalance(int amount) {
     balance += amount;
   } 
};

If this in void Account::Accept() had been a ParentAccount, then the empty function Visitor::Visit(ParentAccount*) would have been called since this function is not overridden in AddToBalance.

Now, we no longer need to define an addToBalance function in ParentAccount:

class ParentAccount : public Account{
   virtual int getBalance() {
      int result = 0;
      for (int i = 0; i < children.size(); i++)
          result += children[i]->getBalance();
      return result;
   }
   //no addToBalance function
};

The second most fun part is this: since we have a tree, we can have a generic function defining a visit sequence, which decides in which order to visit the nodes of the tree:

void VisitWithPreOrderTraversal(Account* _node, Visitor* _visitor) {
  _node->Accept(_visitor);
  for(size_t i = 0; i < _node->children.size(); ++i)
    _node->children[i]->Accept(_visitor);

}

int main() {
  ParentAccount* root = GetRootOfAccount(...);

  AddToBalance* atb = new AddToBalance("pensky_account", 500);
  VisitWithPreOrderTraversal(atb, root);

};

The MOST fun part is defining your own Visitor that does much more complex operations (e.g. accumulating the sums of balances of all ChildAccounts only):

class CalculateBalances : public Visitor {

  void Visit(ChildAccount* _child) {

    balanceSum += _child->getBalance();

  }
  int CumulativeSum() {return balanceSum; }
  int balanceSum;
}

Answer 3

Conceptually, you do not have child and parent accounts, but accounts and a tree of objects, of which the leaf nodes contain a pointer to the actual accounts.

I would suggest you directly represent this structure in code:

class Account
{
public:
    int getBalance(); 
    void addToBalance(int amount);
// privates and implementation not shown for brevity
};


class TreeNode
{
public:
    // contains account instance on leaf nodes, and nullptr otherwise.
    Account* getAccount(); 

    // tree node members for iteration over children, adding/removing children etc

private:
    Account* _account; 
    SomeContainer _children
};

If you now want to traverse the tree to collect account balances etc, you can do it directly on the tree structure. This is simpler and less confusing that taking the route over parent accounts. In addition, it's clear that the actual accounts and the tree structure containing them are different things.