CXXGraph icon indicating copy to clipboard operation
CXXGraph copied to clipboard

Support default serialization for pointer types

Open nrkramer opened this issue 2 years ago • 1 comments

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: https://github.com/ZigRazor/CXXGraph/blob/7c03e1a5a69cd9ad6f141e1cd0efe08ce279ff63/include/Graph/Graph.hpp#L1011 due to this line: https://github.com/ZigRazor/CXXGraph/blob/7c03e1a5a69cd9ad6f141e1cd0efe08ce279ff63/include/Graph/Graph.hpp#L1034

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 ?

nrkramer avatar Sep 04 '23 03:09 nrkramer

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<</>>.

ZigRazor avatar Sep 04 '23 07:09 ZigRazor