Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support default serialization for pointer types #339

Open
nrkramer opened this issue Sep 4, 2023 · 1 comment
Open

Support default serialization for pointer types #339

nrkramer opened this issue Sep 4, 2023 · 1 comment
Labels
core something about core hacktoberfest hacktoberfest issue

Comments

@nrkramer
Copy link
Collaborator

nrkramer commented Sep 4, 2023

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:

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

due to this line:
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 ?

@nrkramer nrkramer added the core something about core label Sep 4, 2023
@ZigRazor
Copy link
Owner

ZigRazor commented Sep 4, 2023

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 ZigRazor added the hacktoberfest hacktoberfest issue label Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core something about core hacktoberfest hacktoberfest issue
Projects
None yet
Development

No branches or pull requests

2 participants