Does it "break OO" to use struct instead of class here?

[mike3]

Hi.

I was wondering about this. I've got this:

template<class V, class E>
class Graph {
 private:
  /* Node object */
  class Edge;
  class Node;

  typedef std::list<Node> NodeList;
  typedef typename std::list<Node>::iterator NodeListIterator;

  typedef std::list<Edge> EdgeList;
  typedef typename std::list<Edge>::iterator EdgeListIterator;

  class Node {
  private:
    V obj;                           /* Object contained in this node */
    std::vector<EdgeListIterator> edgeIters; /* Iterators to edges
					      * connected to this
					      * node.
					      */
  public:
    Node();                          /* Default constructor */
    Node(const V &);                 /* Construct with specified object */
    ~Node();                         /* Destructor */
    V accessObject() const;          /* Access the contained object */
    V& accessObject();
    std::size_t getNumEdges();       /* Get number of edges */
    void addEdgeIter(EdgeListIterator); /* Add an edge iterator */
    XXXXXError alterEdgeIter(std::size_t, EdgeListIterator); /* Alter an edge
							      * iterator
							      */
    EdgeListIterator getEdgeIter(std::size_t); /* Get an edge iterator */
    XXXXXError removeEdgeIter(std::size_t); /* Remove an edge iterator */
  };

  class Edge {
  private:
    E obj;                           /* Object contained in this edge */
    bool isDirected;                 /* Is this edge directed? A directed
				      * edge points from A to B.
				      */
    NodeListIterator A, B;           /* Nodes connected by this edge */
  public:
    Edge();                          /* Default constructor */
    Edge(const E &, bool,
	 NodeListIterator,
	 NodeListIterator);          /* Construct with specified parameters */
    ~Edge();                         /* Destructor */
    E accessObject() const;          /* Access the contained object */
    E& accessObject();
    bool getDirectedness();          /* Get directedness */
    NodeListIterator getAIter();      /* Get node A's iterator */
    NodeListIterator getBIter();      /* Get node B's iterator */
    void alterAIter(NodeListIterator);/* Alter node A's iterator */
    void alterBIter(NodeListIterator);/* Alter node B's iterator */
  };

  /* Node and edge lists */
  NodeList nodes;                    /* Nodes */
  EdgeList edges;                    /* Edges */
 public:
 (... routines for Graph ...)
};

The question comes from how that I have the internal types "Node" and "Edge" as classes with accessor/mutator functions in them. Is this extra complexity needed, or would it be OK practice to just make them "structs" with just data fields in there? As in other places where I've seen something like this done, I often see the equivalent items represented as data-only structs. I'm curious because I'm about to rewrite this thing as it isn't serving me well in its current form, and I was wondering if this is something I should amend or not.

[Bench82]

There's nothing inherently wrong with using a simple struct to represent simple data (In fact, the C++ standard library does this in several places - for example, std::pair).

Your inner classes are implementation details of your outer class anyway, so you're not breaking any part of the outer class' encapsulation (which might be more of a concern).

OO guidelines such as "all data should be private" are best practice in the general case, and are intended to apply to all programming languages which support OO abstraction - however the guidelines don't need to be treated as 'one size fits all' for every conceivable scenario (And C++ is not a "pure OO language" by any stretch either); so if by your judgement you think that using a struct would provide you a clean, simple solution, then I can't really see a problem.

[ne555]

¿Why do you need the accessors for? It seems that you are putting the logic in the wrong place.

  class Edge {
//...
    bool getDirectedness();          /* Get directedness */
    NodeListIterator getAIter();      /* Get node A's iterator */
    NodeListIterator getBIter();      /* Get node B's iterator */
    void alterAIter(NodeListIterator);/* Alter node A's iterator */
    void alterBIter(NodeListIterator);/* Alter node B's iterator */
  };

I want to know how are you using those.
By instance ¿how do you find if 2 nodes are neighbours?

The obj is another issue ¿what is it purpose?

[BlackSheep]

In my opinion having classes that simply provide encapsulation for a single object and accessors to that object are completely unnecessary as they simply add another layer of complexity and overhead. I believe that things like Nodes and Edges (Just arbitrary collections of data) should be contained in "Structs" and not "Classes" unless they actually require functions of their own. (Such as a Node in a binary tree which holds information about how to recursively traverse the tree). I think that the insane amount of encapsulation some people use in C++ is major overkill.
As a side note, structs and classes are the exact same thing (structs can have their own functions), but structs are public by default, and classes are private. The only difference between the two. We're just used to using structs for collections of data, and classes for "Actual Classes".

[mike3]

@BlackSheep: But they do contain their list of edges, which can be used to traverse the graph (get a node's edges, get an edge, follow it to the other node, etc.). So does that cross the line to needing "functions of their own"?

[mike3]

So what would be some good guidelines for when something should be a struct with public members and no functions (except maybe "default" constructors or something to give default values, at best), vs. a full class with hidden data and member functions?

[1yR4jE8b]

Why would it matter? They are pretty much the same thing, it's only syntax/default-visibility issue:

struct Foo {
  //public members here
  protected:
  //protected members here
  private:
  //private members here
};

class Foo {
  //private members here
  protected:
  //protected members here
  public:
  //public members here
};

[mike3]

Yeah, what I mean is when are the appropriate circumstances to choose the all-public, no-functions thing vs. the all-private with-functions one?

[Galik]

I think it is fine to use a struct as an aggregate data type. Go with what seems most natural for a given situation. There is no point gratuitously making everything object oriented. If the data is simple then treat it as data, like you would an int.

After all there comes a point of diminishing returns. We don't create a class Integer every time we want an int. Not that a class Integer can't have its uses, just that it is not always appropriate.

[1yR4jE8b]

But C++ structs have member functions also, the *only* thing that is different is the default-visibility. The question is a no starter, really. It doesn't matter.

[mike3]

@darkestfright: The gist of the question, though, and perhaps I didn't get this across right, was whether that that stuff needed to be hidden behind member functions or not, i.e. whether or not one could get away with just leaving it as raw data to be operated on by Graph.

[coder777]

Yeah, what I mean is when are the appropriate circumstances to choose the all-public, no-functions thing vs. the all-private with-functions one?

It makes sense when the data is treated in very different ways.

For example: You have a server that receives the data converts and passes it to a handler (which is a gui in this case in order to show it). You don't want that your server world is intruded by the gui just because of the data (and vice versa).
Inheritance will make more trouble than it's worth in this case. So leave the data 'naked' and treat it the one or the other way. I find this compromise acceptable. Especially then you're not limited in the ways how to treat the data