Return to Answer

###Public Implementation: Bad

No. You are basically allowing intrusion into the implementation details to implement anything.

struct Graph {
 vector<const Node<T>* > nodes;

Basically you are going to cause headaches for your self down the road because you have expose the implementation details of how the graphs stores its data. People now have to use this to write their functions.

Lock down your member vaiables and provide access to the class via methods to add nodes and edges.

###Visitor Pattern You should also look up the visitor pattern. This allows people to write graph traversal algorithms without knowing details about the graph.

###Stop using using

using std::map;
using std::vector;
using std::cout;
using std::endl;
using std::queue;

You are going to break somebody else code by putting this in the header file. I don't like you using this in a source file (unless you scope it very closely). But putting this in a header file is a define no-no. If I include your header file in my source file you can potentially cause breaking changes in my code that are nearly undetectable. As a result you will find most project will refuse to use your code because of the potential danger to their code base.

If you don't mind messing your own code up (obviously your don't) put it inside your own namespace so at least it does not affect my code.

###Parameters

Use them correctly.

Graph& AddNode(const Node<T>& input) {
 nodes.push_back(&input);
 return *this;
}

So you can pass a node by reference. Which means that internally you will make a copy. Which makes this redundant:

Node<char> a('a');
Node<char> b('b');
Node<char> c('c');
Node<char> d('d');
Node<char> e('e');
Node<char> f('f');
graph.AddNode(a).AddNode(b).AddNode(c).AddNode(d).AddNode(e).AddNode(f);

You now have two copies of each node.

Would be easier to write like this:

graph.AddNode(Node<char>('a')).AddNode(Node<char>('b')).AddNode(Node<char>('c')).AddNode(Node<char>('d')).AddNode(Node<char>('e')).AddNode(Node<char>('f'));

But that seems a bit verbose. So you could have written a helper function that does type deduction for you.

template<typename T>
Node<T> mkNode(T const& v) {return Node<T>(v);}
 graph.AddNode(mkNode('a')).AddNode(mkNode('b')).AddNode(mkNode('c')).AddNode(mkNode('d')).AddNode(mkNode('e')).AddNode(mkNode('f'));

Or even better just write an AddNode() that takes a T and internall creates a Node of the correct type.

graph.AddNode('a').AddNode('b').AddNode('c').AddNode('d').AddNode('e').AddNode('f');

Public Implementation: Bad

No. You are basically allowing intrusion into the implementation details to implement anything.

struct Graph {
 vector<const Node<T>* > nodes;

Basically you are going to cause headaches for your self down the road because you have expose the implementation details of how the graphs stores its data. People now have to use this to write their functions.

Lock down your member vaiables and provide access to the class via methods to add nodes and edges.

Visitor Pattern

You should also look up the visitor pattern. This allows people to write graph traversal algorithms without knowing details about the graph.

Stop using using

using std::map;
using std::vector;
using std::cout;
using std::endl;
using std::queue;

You are going to break somebody else code by putting this in the header file. I don't like you using this in a source file (unless you scope it very closely). But putting this in a header file is a define no-no. If I include your header file in my source file you can potentially cause breaking changes in my code that are nearly undetectable. As a result you will find most project will refuse to use your code because of the potential danger to their code base.

If you don't mind messing your own code up (obviously your don't) put it inside your own namespace so at least it does not affect my code.

Parameters

Use them correctly.

Graph& AddNode(const Node<T>& input) {
 nodes.push_back(&input);
 return *this;
}

So you can pass a node by reference. Which means that internally you will make a copy. Which makes this redundant:

Node<char> a('a');
Node<char> b('b');
Node<char> c('c');
Node<char> d('d');
Node<char> e('e');
Node<char> f('f');
graph.AddNode(a).AddNode(b).AddNode(c).AddNode(d).AddNode(e).AddNode(f);

You now have two copies of each node.

Would be easier to write like this:

graph.AddNode(Node<char>('a')).AddNode(Node<char>('b')).AddNode(Node<char>('c')).AddNode(Node<char>('d')).AddNode(Node<char>('e')).AddNode(Node<char>('f'));

But that seems a bit verbose. So you could have written a helper function that does type deduction for you.

template<typename T>
Node<T> mkNode(T const& v) {return Node<T>(v);}
 graph.AddNode(mkNode('a')).AddNode(mkNode('b')).AddNode(mkNode('c')).AddNode(mkNode('d')).AddNode(mkNode('e')).AddNode(mkNode('f'));

Or even better just write an AddNode() that takes a T and internall creates a Node of the correct type.

graph.AddNode('a').AddNode('b').AddNode('c').AddNode('d').AddNode('e').AddNode('f');

###Public Implementation: Bad

No. You are basically allowing intrusion into the implementation details to implement anything.

struct Graph {
 vector<const Node<T>* > nodes;

Basically you are going to cause headaches for your self down the road because you have expose the implementation details of how the graphs stores its data. People now have to use this to write their functions.

Lock down your member vaiables and provide access to the class via methods to add nodes and edges.

###Visitor Pattern You should also look up the visitor pattern. This allows people to write graph traversal algorithms without knowing details about the graph.

###Stop using using

using std::map;
using std::vector;
using std::cout;
using std::endl;
using std::queue;

You are going to break somebody else code by putting this in the header file. I don't like you using this in a source file (unless you scope it very closely). But putting this in a header file is a define no-no. If I include your header file in my source file you can potentially cause breaking changes in my code that are nearly undetectable. As a result you will find most project will refuse to use your code because of the potential danger to their code base.

If you don't mind messing your own code up (obviously your don't) put it inside your own namespace so at least it does not affect my code.

###Public Implementation: Bad

No. You are basically allowing intrusion into the implementation details to implement anything.

struct Graph {
 vector<const Node<T>* > nodes;

Basically you are going to cause headaches for your self down the road because you have expose the implementation details of how the graphs stores its data. People now have to use this to write their functions.

Lock down your member vaiables and provide access to the class via methods to add nodes and edges.

###Visitor Pattern You should also look up the visitor pattern. This allows people to write graph traversal algorithms without knowing details about the graph.

###Stop using using

using std::map;
using std::vector;
using std::cout;
using std::endl;
using std::queue;

You are going to break somebody else code by putting this in the header file. I don't like you using this in a source file (unless you scope it very closely). But putting this in a header file is a define no-no. If I include your header file in my source file you can potentially cause breaking changes in my code that are nearly undetectable. As a result you will find most project will refuse to use your code because of the potential danger to their code base.

If you don't mind messing your own code up (obviously your don't) put it inside your own namespace so at least it does not affect my code.

###Parameters

Use them correctly.

Graph& AddNode(const Node<T>& input) {
 nodes.push_back(&input);
 return *this;
}

So you can pass a node by reference. Which means that internally you will make a copy. Which makes this redundant:

Node<char> a('a');
Node<char> b('b');
Node<char> c('c');
Node<char> d('d');
Node<char> e('e');
Node<char> f('f');
graph.AddNode(a).AddNode(b).AddNode(c).AddNode(d).AddNode(e).AddNode(f);

You now have two copies of each node.

Would be easier to write like this:

graph.AddNode(Node<char>('a')).AddNode(Node<char>('b')).AddNode(Node<char>('c')).AddNode(Node<char>('d')).AddNode(Node<char>('e')).AddNode(Node<char>('f'));

But that seems a bit verbose. So you could have written a helper function that does type deduction for you.

template<typename T>
Node<T> mkNode(T const& v) {return Node<T>(v);}
 graph.AddNode(mkNode('a')).AddNode(mkNode('b')).AddNode(mkNode('c')).AddNode(mkNode('d')).AddNode(mkNode('e')).AddNode(mkNode('f'));

Or even better just write an AddNode() that takes a T and internall creates a Node of the correct type.

graph.AddNode('a').AddNode('b').AddNode('c').AddNode('d').AddNode('e').AddNode('f');