-
Notifications
You must be signed in to change notification settings - Fork 41
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
Tree parsing and overhaul #38
Comments
Hey @BenjaBobs, Wow! You have been busy. I seem to remember some very specific reason why I chose to use abstract classes instead of interfaces, but I can't for the life of me remember why that was. Please give me a few days (maybe even a week) to digest all of this so that I can compile the feedback you deserve. (I'll also have the other members of my team take a look.) I hope this isn't too long of a time for you. Thanks for all your hard work! Glad to see someone is getting some good use out of this code. |
Thanks for having a look! |
@evo-terren Since PR is merged, wanted to know why is |
I just had a quick look at the code, and I gotta say I don't remember. It might be an oversight. |
Hi @evo-terren
I'm currently working on a way to deserialize trees (much like
Vertex.ToObject<T>()
) since I need it for my project, but it has turned out to be quite complex.With that I also made some changes to how the schema bound graph traversals work, and I'd like your input on it, especially in regard of whether you want it as part of the project.
#39
Anyway here's the gist of it:
Schema changes
Introduction of interfaces:
This way, entities don't have to inherit from
VertexBase
/EdgeBase
to be usable in the traversal.The benefit of this is that it frees up the inheritance chain.
Label changes
Depending on the data model size, the amount of edges can become quite big and as such the amount of classes needed.
To help alleviate this I've introduced property labels:
As well as a set of generic types, ie:
Which enables a schema definition syntax like this:
And since the schema traversal are based on
IVertex
andIEdge<TOutV,TInv>
, it is still possible to create custom edges that implement these interfaces or inhereit from predefined ones such asManyToManyEdge<TOutV, TInV>
.Tree conversion
The trouble of converting trees into specific types are that the edges and vertices need to be connected properly.
For that I made some interfaces and a naive implementation, such that there is an option for adding specialized implementations.
With that follows a default
AutoConnector
, which can make some qualified guesses as to how to merge.I then added the following method to
Tree
:The
TreeParser
does the actual connecting of the vertices and edges, and defaults toAutoConnector
if no other implementations are provided.I updated the
TreeJsonConverter_deserializes_a_tree_that_includes_edges()
test and can confirm that it works, using the schema example from above.The code is currently not the most beautiful in the world, but I was hoping you'd provide some input about the whole thing. 😄
Also sorry about making it such a huge blob of changes, I know it kind of got out of hand.
The text was updated successfully, but these errors were encountered: