Skip to content

Conversation

@To-om
Copy link
Contributor

@To-om To-om commented Feb 24, 2019

This PR adds type safe version of project. This uses a ProjectionBuilder class to build HList type. I choose to not expose projection label (only order of projections counts):

graph
        .V()
        .out("created")
        .project(_
          .apply(_.value(Key[String]("name")))
          .and(_.in("created").count()))
        .head()
// returns "lop" :: 3 :: HNil with correct type

@mpollmeier
Copy link
Owner

This is great, thank you! I'd prefer to not expose HList to the user, instead we should transform it to tuples, which is possible (used e.g. in select). What do you think?

Also note that #241 by @jeremysears is going in a similar direction - I tried to chip in but didn't manage to finish it.

@To-om
Copy link
Contributor Author

To-om commented Feb 26, 2019

I agree with you. HList complexity should not be exposed to user.

@mpollmeier
Copy link
Owner

I just took the liberty to refactor the testcase, let me know if you like it: a141bb4

For the 'simple' case of having only one By modulation, the result type is always derived as a Tuple1, which is clunky to work with - have a look at the test I added. I'm guessing shapeless can map that to the wrapped type, but no idea how clunky that becomes... thoughts?

This is fun!

@To-om
Copy link
Contributor Author

To-om commented Feb 27, 2019

I don't think that Tuple1 is problematic as the use of project for only one projection is non-sense (you can simply remove the project step).
If you really want to unwrap Tuple1, it is possible to add a new layer of implicit that transforms Tuple1[T] into T and other types by themselves. IMHO it adds more useless complexity.

@mpollmeier
Copy link
Owner

🤦‍♂️ of course, you're right.

I dropped that nonsense test, moved ProjectionBuilder to it's own file: #273
I'll merged to master once CI is green, then it will release automagically. Thank you!

@mpollmeier mpollmeier closed this Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants