ZigRazor/CXXGraph

Support default serialization for pointer types

Opened this issue · 1 comments

nrkramer commented

Some library users may want to use pointer types in Graph templates, such as:

CXXGraph::Graph<MyCustomType*> graph;

The code in Graph.hpp that prevents compiling without defining stream operators on the type (such as operator <<() and operator >>()) is in a few places, here's one:

CXXGraph/include/Graph/Graph.hpp

Line 1011 in 7c03e1a

void Graph<T>::readGraphFromStream(std::istream &iGraph,

due to this line:

CXXGraph/include/Graph/Graph.hpp

Line 1034 in 7c03e1a

while (iNodeFeat >> nodeId >> nodeFeat) {

And an example compiler error:

E:\Development\CXXGraph\include\Graph/Graph.hpp(1034,22): error C2678: binary '>>': no operator found which takes a left-hand operand of type 'std::basic_istream<char,std::char_traits<char>>' (or there is no acceptable conversion) [E:\Development\CXXGraph\build\test\test_exe.vcxproj]

E:\Development\CXXGraph\include\Graph/Graph.hpp(1034,22): message : 'std::basic_istream<_Elem,_Traits> &std::operator >>(std::basic_istream<_Elem,_Traits> &,std::basic_string<_Elem,_Traits,_Alloc> &)': could not deduce template argument for 'std::basic_string<_Elem,_Traits,_Alloc> &' from 'T' [E:\Development\CXXGraph\build\test\test_exe.vcxproj]
          with
          [
              T=testClass *
          ]

Here's the example code that I used to generate that:

class testClass {
  public:
    std::vector<int> container;

    testClass() = default;
};

...

TEST(GraphTest, Constructor_custom) {
  CXXGraph::Node<testClass*> node1("1", nullptr);
  CXXGraph::Node<testClass*> node2("2", nullptr);
  CXXGraph::Edge<testClass*> edge(1, node1, node2);

  CXXGraph::T_EdgeSet<testClass*> edgeSet;
  edgeSet.insert(make_shared<CXXGraph::Edge<testClass*>>(edge));

  CXXGraph::Graph<testClass*> graph(edgeSet);
  ASSERT_EQ(graph.getEdgeSet(), edgeSet);
}

Discussion

We should consider providing some kind of default function based on pointer type-inference that:

  1. Warn the user they may be attempting to (de)serialize a pointer object without a stream operator
  2. Serializes either the id, or pointer address (up for discussion on this point)

Many users may be enjoying CXXGraph for its ease of in-memory graph manipulation, and not care about serder (Serialization/Deserialization). If they do care about serder, they can heed the compiler warning and implement stream operators for their pointer types.

FWIW, I think we shouldn't prevent users from compiling even with non-pointer types that don't have stream operators. Warning the user, and even completely compiling out the streaming functions when the user uses a non-stream-operator-friendly class seems like a more scalable approach.

WDYT @sbaldu @ZigRazor ?

ZigRazor commented

I agree with you. We should provide a simple warning while try to call a specific function for serialize/deseriale pointer for example a friend operator<</>>.

  • 👍1