class design - use of friends / getter(setter) / public : Lesser Evil

[wtf4096]

I`m designing a series of classes that are responsible for creation of a lego-style (well, sort-of) 3D meshes. The basic building block of each mesh is a Box. There could be well over 1000 of these Boxes in each 3D mesh.

However, my following design looks dirty to me since I had to add several "friend class" lines to compile it. How can I clean it and make it less dirty ? Since it`s a relatively common scenario, I`d like to know how to do it properly in the future.

AFAIK, I could have just made the base building block (CBox) public and it would get me rid of "friend class" declarations. But would it really be dirty, if this CBox is really just a simple basic building block ? It could even be a "struct"
Another alternative would be specifying getter/setter methods which would just clutter the interface, so I scrapped this idea.

I`ve had a heated debate with the colleagues on this one and it seems that everybody considers "cluttered/dirty" in a different way.

On the other hand, I personally think, that this is one of the proper uses of "friend" keyword, since it`s 100% sure that only relevant classes get access to this base building block, thus they can`t break things up.
What do you think ?

//  Forward declarations
class CScene;

    //  Mesh class
class CMesh
    {
  private:
    friend class CScene;
    class CBox                  
        {
        friend class CMesh;
        friend class CScene;
        class CColor
            {
            friend class CBox;
            D3DCOLOR RGBValue;  

            CColor (unsigned char R = 255, unsigned char G = 255, unsigned char B = 255) : RGBValue (D3DCOLOR_RGBA (R,G,B,255)) {};
            void    Set (unsigned char R, unsigned char G, unsigned char B)
                        {   RGBValue = D3DCOLOR_RGBA (R,G,B,255);   }
            };

        D3DXVECTOR3 POrigin;                        
        D3DXVECTOR3 PWidth, PDepth, PHeight;        
        CColor  Color;                              

        CBox (D3DXVECTOR3 fPOrigin = D3DXVECTOR3(0,0,0), D3DXVECTOR3 fPWidth = D3DXVECTOR3(0,0,0), D3DXVECTOR3 fPDepth = D3DXVECTOR3(0,0,0), D3DXVECTOR3 fPHeight = D3DXVECTOR3(0,0,0), CColor fColor = CColor ()) : POrigin (fPOrigin), PWidth (fPWidth), PDepth (fPDepth), PHeight (fPHeight), Color (fColor)    {};
        void    Set (D3DXVECTOR3 fPOrigin, D3DXVECTOR3 fPWidth, D3DXVECTOR3 fPDepth, D3DXVECTOR3 fPHeight, CBox::CColor fColor)
                    {  POrigin = fPOrigin; PWidth = fPWidth; PDepth = fPDepth; PHeight = fPHeight; Color = fColor;  };
        void    Set (D3DXVECTOR3 fPOrigin, D3DXVECTOR3 fPWidth, D3DXVECTOR3 fPDepth, D3DXVECTOR3 fPHeight)
                    {  POrigin = fPOrigin; PWidth = fPWidth; PDepth = fPDepth; PHeight = fPHeight; };
        };

    CBox    BBox;               
    vector <CBox> Boxes;        

  public:
    void    AddHorizontalBars (int BarsCount, float BarSpacing, D3DXVECTOR3 BarOrigin, float BarWidth, float BarDepth, float BarHeight, CBox::CColor BarCOlor);
    void    AddVerticalBars (int BarsCount, float BarSpacing, D3DXVECTOR3 BarOrigin, float BarWidth, float BarDepth, float BarHeight, CBox::CColor BarCOlor);

    CMesh ()    {   };
    ~CMesh ()    {   void ReleaseAllMemory ();  };
    void ReleaseAllMemory () {  };
    };


    //  Class for Scene containing all meshes
class CScene
    {
  private:
    vector <CMesh> Meshes;    //  Array of Scene meshes
    void    GenerateMeshes ();

  public:
    CScene ()    {   };
    ~CScene ()   {   ReleaseAllMemory ();    }
    void ReleaseAllMemory () {  };
    };

[Radical Edward]

Ed never liked nested classes. They feel inelegant to me, especially when data access is much more important than class visibility and it's easy to restrict class visibility to a single file:

// Assume you don't want CColor or CBox
// to be visible outside of this file
namespace {
  class CColor {
    D3DCOLOR RGBValue;  
  public:
    CColor();
    void Set();
  };

  class CBox {
    D3DXVECTOR3 POrigin;                        
    D3DXVECTOR3 PWidth;
    D3DXVECTOR3 PDepth;
    D3DXVECTOR3 PHeight;        
    CColor Color; 
  public:
    CBox();
    void Set();
    void Set();
  };
}

// CMesh and CScene are visible globally
// and because they're in the same file,
// they can see both CColor and CBox
class CMesh {
  CBox BBox;               
  vector<CBox> Boxes; 
public:
  CMesh();
  ~CMesh();
  void AddHorizontalBars();
  void AddVerticalBars();
  void ReleaseAllMemory();
};

class CScene {
  vector<CMesh> Meshes;
  void GenerateMeshes();
public:
  CScene();
  ~CScene();
  void ReleaseAllMemory();
};

[wtf4096]

Well, CMesh and CScene actually can`t see CBox and CColor. I have to use a global resolution specifier "::" if I want to access them inside CMesh/CScene.

I`m not sure now - was it intentional from your side to end the empty namespace so that it contains just those 2 classes - CColor and CBox ? Probably yes, since otherwise even CMesh and CScene wouldn`t be visible globally.

[wtf4096]

Damn. I don`t how could I have overlooked it, but in my first implementation, the constructors of CBox and CColor (and setters) were private !

Then, it was easy : No empty namespaces, no hacks, and no resulting friend declarations. Actually, it`s one of the rare right uses of multiple private/public blocks within the declaration (the CColor must be declared as first and as public, otherwise CBox constructor won`t recognize it. Thus, we must switch back to private section to declare CBox`s members.).
As an added bonus, no forward declarations are needed, so it`s even less cluttered.
Here it is :

class CBox                  
        {
      public:
        class CColor
            {
            D3DCOLOR RGBValue;  
          public:
            CColor (unsigned char R = 255, unsigned char G = 255, unsigned char B = 255) : RGBValue (D3DCOLOR_RGBA (R,G,B,255)) {};
            void    Set (unsigned char R, unsigned char G, unsigned char B)
                        {   RGBValue = D3DCOLOR_RGBA (R,G,B,255);   }
            };
      private:
        D3DXVECTOR3 POrigin;                        
        D3DXVECTOR3 PWidth, PDepth, PHeight;        
        CColor  Color;                              
      public:
        CBox (D3DXVECTOR3 fPOrigin = D3DXVECTOR3(0,0,0), D3DXVECTOR3 fPWidth = D3DXVECTOR3(0,0,0), D3DXVECTOR3 fPDepth = D3DXVECTOR3(0,0,0), D3DXVECTOR3 fPHeight = D3DXVECTOR3(0,0,0), CColor fColor = CColor ()) : POrigin (fPOrigin), PWidth (fPWidth), PDepth (fPDepth), PHeight (fPHeight), Color (fColor)    {};
        void    Set (D3DXVECTOR3 fPOrigin, D3DXVECTOR3 fPWidth, D3DXVECTOR3 fPDepth, D3DXVECTOR3 fPHeight, CBox::CColor fColor)
                    {  POrigin = fPOrigin; PWidth = fPWidth; PDepth = fPDepth; PHeight = fPHeight; Color = fColor;  };
        void    Set (D3DXVECTOR3 fPOrigin, D3DXVECTOR3 fPWidth, D3DXVECTOR3 fPDepth, D3DXVECTOR3 fPHeight)
                    {  POrigin = fPOrigin; PWidth = fPWidth; PDepth = fPDepth; PHeight = fPHeight; };
        };

    //  Mesh class
class CMesh
    {
  private:
    CBox    BBox;               
    vector <CBox> Boxes;        

  public:
    void    AddHorizontalBars (int BarsCount, float BarSpacing, D3DXVECTOR3 BarOrigin, float BarWidth, float BarDepth, float BarHeight, CBox::CColor BarCOlor);
    void    AddVerticalBars (int BarsCount, float BarSpacing, D3DXVECTOR3 BarOrigin, float BarWidth, float BarDepth, float BarHeight, CBox::CColor BarCOlor);

    CMesh ()    {   };
    ~CMesh ()    {   void ReleaseAllMemory ();  };
    void ReleaseAllMemory () {  };
    };


    //  Class for Scene containing all meshes
class CScene
    {
  private:
    vector <CMesh> Meshes;    //  Array of Scene meshes
    void    GenerateMeshes ();

  public:
    CScene ()    {   };
    ~CScene ()   {   ReleaseAllMemory ();    }
    void ReleaseAllMemory () {  };
    };

Sometimes you just have to go out of the computer for a while, so it seems :-)

[Radical Edward]

> Well, CMesh and CScene actually can`t see CBox and CColor.
In the snippet Edward gave, certainly not if CMesh and CScene are defined in a different from from CBox and CColor. That's what an unnamed namespace does, restrict visibility of its contents to the source file.

> was it intentional from your side to end the empty
> namespace so that it contains just those 2 classes
Yes, Ed was giving an example of how to restrict the visibility of classes to the file, if that's even what you need.

[wtf4096]

I didn`t know the trick with unnamed namespace. It`s a great technique since I`ve already experienced many times that I wanted to name some class with the same logical name, but wanted it to be just local to the current header file.

So far, the only way for me was to use nested classes. But now I can use this "empty namespace" technique, that you have just shown ;-)