Library to analyse corporate ownership

Question

I've begun developing a library which can be used to represent and analyse corporate ownership structures. The first stage has been to design a library which allows a user to model ownership structures (e.g. company A owns 52% of B and 90% of C).

The library contains two fundamental types - entities (i.e. companies) and entityspace (where different entities exist). Each entity has a vector of 'relations' which relate that entity to another entity via an ownership relation (e.g. a relation might designate A as a parent of B with 50 'units' of ownership). Each relation is mirrored in the related entity's relations vector (e.g. if A has a relation which shows it has 50 units in B then B will have a relation showing it has 50 units owned by A).

I'd be grateful for any comments on this library, especially the design (e.g. is it ok for each entity object to refer back to the entityspace,should the 'link' functions in the entity class actually belong to entityspace, am I returning the appropriate types).

Thanks

main.cpp:

#include <iostream>
#include <map>

#include "Entityspace.h"

using namespace std;


int main()
{
  Entityspace test;
  EntityID father = test.addEntity("Father Ltd", EType::LTD);
  EntityID mother = test.addEntity("Mother Plc", EType::PLC);
  EntityID son = test.addEntity("Son LLP", EType::LLP);
  EntityID daughter = test.addEntity("Daughter", EType::IND);

  test[father].addEntAsChild(son,40, UType::ORDINARY_SHARE);
  test[father].addEntAsChild(daughter,30, UType::ORDINARY_SHARE);

  test[son].addEntAsParent(mother,50, UType::ORDINARY_SHARE);
  test[daughter].addEntAsParent(mother,60, UType::ORDINARY_SHARE);

  test.deleteEntity(mother);

  VecEID ents = test.findParentsOf(son);

  return 0;
}

Entityspace.h:

#ifndef ENTITYSPACE_H
#define ENTITYSPACE_H

#include <map>

#include "Entity.h"

using namespace std;

class Entityspace
{
private:
  map<EntityID,Entity> EntityContainer;
  VecEID findDirectRelations(EntityID id, RType rel_type) const; //function used by findParentsOf and findChildrenOf

  public:
  Entity& operator[](EntityID id);
  const Entity& operator[](EntityID id) const; //const qualified version

  EntityID addEntity(const string& nm, EType type);
  bool deleteEntity(EntityID id);

  bool hasEntity (EntityID Id) const;
  int totalEntsCount() const;

  //methods of finding related entities
  VecEID findParentsOf(EntityID id) const;
  VecEID findChildrenOf(EntityID id) const;
  VecEID findSiblingsOf(EntityID id) const;
};

#endif

Entityspace.cpp:

#include <algorithm>
#include "Entityspace.h"

//private member functions

VecEID Entityspace::findDirectRelations(EntityID Id,RType rtype) const
{
  VecEID relatedEnts;

  const Entity& ent = (*this)[Id];
  VecRel foundRels = ent.findRelsByType(rtype);

  for (VecRel_It it = foundRels.begin(); it != foundRels.end();it++)
  {
    if (find(relatedEnts.begin(), relatedEnts.end(),it->related_ent_id) == relatedEnts.end()) //do not add duplicate parents
    {
      relatedEnts.push_back(it->related_ent_id);
    }
  }
  return relatedEnts;
}

Entity& Entityspace::operator[](EntityID Id)
{
  return EntityContainer.at(Id);
}

const Entity& Entityspace::operator[](EntityID Id) const
{
  return EntityContainer.at(Id);
}

//public member functions

EntityID Entityspace::addEntity(const string& nm,EType type)
{
  Entity ent(nm,type, *this);
  EntityContainer.insert(pair<EntityID,Entity>(ent.getId(),ent));

  return ent.getId();
}

bool Entityspace::deleteEntity(EntityID Id)
{
  if (!hasEntity(Id))  //cannot delete entity if it does not exist
  {
    return false;
  }

  Entity& entToDel = (*this)[Id];
  VecRel entToDelRels = entToDel.getRelsVec(); //all relations of the entity to be deleted

  for (VecRel::const_iterator it = entToDelRels.begin(); it != entToDelRels.end();++it)
  {
      if (!entToDel.deleteRelation(*it)) //delink every relation in the entity being deleted (so that equivalent relations removed from related entities)
      {
        return false; //if a delink fails then PROBLEM that other relations already deleted
      }
  }
  EntityContainer.erase(Id); //remove the entity from Entityspace
  return true;
}

bool Entityspace::hasEntity(EntityID Id) const
{
  return (EntityContainer.find(Id) != EntityContainer.end());
}

int Entityspace::totalEntsCount() const
{
  return EntityContainer.size();
}

VecEID Entityspace::findParentsOf(EntityID Id) const
{
  return findDirectRelations(Id, RType::OWNED_BY);
}

VecEID Entityspace::findChildrenOf(EntityID Id) const
{
  return findDirectRelations(Id, RType::OWNER_OF);
}

VecEID Entityspace::findSiblingsOf(EntityID Id) const
{
  VecEID sibEntsArr;

  VecEID prtEnts = findParentsOf(Id);// all parents
  for (VecEID_It prtIt = prtEnts.begin(); prtIt != prtEnts.end();prtIt++)
  {
    VecEID prtCldEnts = findChildrenOf(*prtIt); //all children of each parent
    for (VecEID_It prtCldIt = prtCldEnts.begin(); prtCldIt != prtCldEnts.end();prtCldIt++)
    {
      if (*prtCldIt == Id) // do not add a the current entity
      {
        continue;
      }

      if (find(sibEntsArr.begin(),sibEntsArr.end(),*prtCldIt) != sibEntsArr.end()) //do not add duplicate siblings
      {
        continue;
      }
      sibEntsArr.push_back(*prtCldIt);
    }
  }
  return sibEntsArr;
}

Entity.h:

#ifndef ENTITY_H
#define ENTITY_H

#include <string>
#include <vector>
#include <map>

using namespace std;

class Entityspace;   //forward declaration

enum class RType
{
  OWNER_OF,
  OWNED_BY
};

enum class EType{   //type of entity
  LTD,
  LLP,
  PLC,
  IND
};

enum class UType{ //type of unit
  ORDINARY_SHARE,
  PREFERENCE_SHARE,
  VOTING_RIGHT,
  LLP_MEMBER
};

struct relation;  //forward declaration

typedef vector<relation> VecRel;
typedef vector<relation>::const_iterator VecRel_It;

typedef unsigned int EntityID;
typedef vector<EntityID> VecEID;
typedef vector<EntityID>::const_iterator VecEID_It;

class Entity
{
  private:
    //private variables
    static int idgenerator;  //used to generate 'unique' ids for Entity objects

    string name;
    EntityID ent_id;
    EType type;
    VecRel rels_v; //stores all relations with other objects (i.e. as parent/child) - these relations are mirrored in related Entity. There may be identical relations stored in this vector.

    Entityspace& rEntityContainer; //reference to container - this allows Entity methods to access other Entities through the Entityspace map

    //private member functions - can still be accessed by other Entity objects
    void addRelToVec(relation rel);
    void delRelFrVec(VecRel_It rel); //deletes relation from relations vector
    VecRel_It findFirstRel(relation rel); //return iterator to first matching relation in relsvec
    VecRel_It relsVecEnd(); //iterator to end of relsvec

  public:
    Entity (string value1, EType value2, Entityspace& ES_r);

    //getter/setters
    void setName(string name);
    string getName() const;
    EntityID getId() const;

    //linkage functions
    bool addRelation(relation rel); //this adds a relation to rels_v
      bool addEntAsParent(EntityID id, unsigned int units, UType units_type); //convenience function
      bool addEntAsChild(EntityID id, unsigned int units, UType units_type); //convenience function
    bool deleteRelation(relation rel); //remove first matching relation from rels_v

    //find functions
    VecRel getRelsVec() const; //this returns a copy of a vector of an Entities' relations

    VecRel findRelsById(EntityID rel_id) const; //returns a vector of relations with matching Id
    VecRel findRelsByType(RType type) const;
    VecRel findRelsByUnits(unsigned int min_units, unsigned int max_units) const;
};

struct relation { //struct containing entity related to, number of ownership units owned and type of relationship i.e. owned by/owner of
  EntityID related_ent_id; //the id of the parent/child
  unsigned int units; //how many units are owned
  UType units_type; //type of unit
  RType rel_type; //OWNER_OF or OWNED_BY

  bool operator==(const relation &rhs) //when we need to compare a relation - think this may redundant
  {
    return (related_ent_id==rhs.related_ent_id && units==rhs.units && units_type==rhs.units_type && rel_type==rhs.rel_type);
  }
};
#endif

Entity.cpp:

#include <algorithm>

#include "Entity.h"
#include "Entityspace.h"   //because we are using methods of Entityspace class which has been forward declared

int Entity::idgenerator = 1;  // initialisation of static idgenerator

//private member functions

void Entity::addRelToVec(relation rel)
{
  rels_v.push_back(rel);
}

void Entity::delRelFrVec(VecRel_It relIt)
{
  rels_v.erase(relIt);
}

VecRel_It Entity::findFirstRel(relation rel)
{
  return find(rels_v.begin(), rels_v.end(), rel);
}

VecRel_It Entity::relsVecEnd()
{
  return rels_v.end();
}

 //public member functions

Entity::Entity (string value1, EType value2, Entityspace& value3): name(value1), type(value2), rEntityContainer(value3)
{
  ent_id = idgenerator;
  idgenerator++;
}

void Entity::setName(string nm)
{
  name = nm;
}

string Entity::getName() const
{
  return name;
}

EntityID Entity::getId() const
{
  return ent_id;
}

bool Entity::addRelation (relation rel)
{
  //check to ensure valid request
  if (rel.related_ent_id == getId())  //cannot link to self
  {
    return false;
  }

  if (!rEntityContainer.hasEntity(rel.related_ent_id)) // cannot find entity to be linked
  {
    return false;
  }

  //valid, so make link
  addRelToVec(rel);

  RType rev_rel_type = static_cast<RType>(1 - static_cast<underlying_type<RType>::type> (rel.rel_type)); //ugly way of flipping enum (as other entity will have opposite relation type)
  rEntityContainer[rel.related_ent_id].addRelToVec(relation{getId(),rel.units,rel.units_type,rev_rel_type}); //add equivalent relation to entity to be linked

  return true;
}

bool Entity::addEntAsParent(EntityID id, unsigned int units, UType units_type)
{
  return addRelation(relation{id,units,units_type, RType::OWNED_BY});
}

bool Entity::addEntAsChild(EntityID id, unsigned int units, UType units_type)
{
  return addRelation(relation{id,units, units_type, RType::OWNER_OF});
}

bool Entity::deleteRelation(relation rel)
{
  //check its ok to delink
  if(rel.related_ent_id == getId()) // cannot delink self
  {
    return false;
  }

  if (!rEntityContainer.hasEntity(rel.related_ent_id)) // cannot find entity to be delinked
  {
    return false;
  }
  Entity& lnkEnt = rEntityContainer[rel.related_ent_id]; //reference to entity to be delinked

  VecRel_It entRelIt = findFirstRel(rel); //get iterator to first matching relation in current ent
  if (entRelIt == relsVecEnd()) //if none found then terminate
  {
    return false;
  }

  relation opp_rel {getId(),rel.units,rel.units_type,static_cast<RType>(1 - static_cast<underlying_type<RType>::type> (rel.rel_type))}; //this is the equivalent relation in the linked object
  VecRel_It lnkEntRelIt = lnkEnt.findFirstRel(opp_rel); //

  if (lnkEntRelIt == lnkEnt.relsVecEnd())
  {
    return false;
  }

  //ok to remove links
  delRelFrVec(entRelIt);
  lnkEnt.delRelFrVec(lnkEntRelIt);

  return true;
}

VecRel Entity::getRelsVec() const
{
  return rels_v;
}

VecRel Entity::findRelsById (EntityID rel_id) const
{
  VecRel relsToReturn;
  for (VecRel_It it = rels_v.begin(); it != rels_v.end();it++)
  {
    if (it->related_ent_id == rel_id)
    {
      relsToReturn.push_back(*it);
    }
  }
  return relsToReturn;
}

VecRel Entity::findRelsByType (RType type) const
{
  VecRel relsToReturn;
  for (VecRel_It it = rels_v.begin(); it != rels_v.end();++it)
  {
    if (it->rel_type == type)
    {
      relsToReturn.push_back(*it);
    }
  }
  return relsToReturn;
}

VecRel Entity::findRelsByUnits (unsigned min, unsigned max) const
{
  VecRel relsToReturn; // vector of iterators to elements in relations vector

  for (VecRel_It it = rels_v.begin(); it != rels_v.end();++it)
  {
    if (it->units >= min && it->units <= max)
    {
      relsToReturn.push_back(*it);
    }
  }
  return relsToReturn;
}

Answer 1

I've not looked over all the code, but here are some observations.

Avoid using namespace std;.

You don't document how you handle not finding a company when you try to look it up with your operator[] function. The expectation (from the standard containers) is that if this method is provided, the existing element will be returned or a new one will be created. No exceptions will be thrown. Your code defers to map's at, which will throw an exception if the EntityId is not found. These should probably be called at instead.

You have inconsistent spacing. The public: in Entityspace is indented and hard to see, and some of your function calls lack spaces after a comma (see, for example, the call to addEntAsChild in main, and your declaration of EntityContainer in Entityspace).

In addEntity, you use map's insert method. However, the pair you pass is not correctly typed. A map has a const first value in the pair, so the correct type is pair<const EntityID, Entity>. Without that const there will be one pair created, then copied to a 2nd pair to convert it to the correct type. A simple way to avoid this is to use the value_type of the map if you have a typedef for it. Better still is to forgo insert and use emplace.

EntityContainer.emplace(ent.getId(), ent);

How does addEntity behave if an entity already exists? Since you're ignoring the result from insert (or emplace), you'll return the wrong Id. It should return the one stored in the map, not the temporary object created for the insertion.

totalEntsCount should return a size_t.

Some of your for loops can be simplified using the range-based for loop. For example, instead of

for (VecEID_It prtIt = prtEnts.begin(); prtIt != prtEnts.end();prtIt++)
{
    VecEID prtCldEnts = findChildrenOf(*prtIt); //all children of each parent

you can use

for (const auto &prt: prtEnts)
{
    VecEID prtCldEnts = findChildrenOf(prt);

Your use of 1 - rtype to change the relation type makes an assumption about the relative difference of OWNER_OF and OWNER_OF. If this ever changes (because you add more complicated relations or different relation types) your code will break in possibly hard to detect ways. A better way to do this is to create a function to call to get the complementary/reverse relation given an existing relationship.

Answer 2

Take this with a grain of salt, because my "modern" C++ is not at all fluent, but: read about this form of for loop.

https://en.cppreference.com/w/cpp/language/range-for

This will allow you to simplify your loops that operate on STL iterators.

Otherwise, your code seems quite clean to me. Consider adding structured documentation, in a format such as the one that Doxygen supports.