Fluent JCascalog #79

Closed
BertrandDechoux opened this Issue May 10, 2012 · 4 comments

Comments

Projects
None yet
3 participants
@BertrandDechoux
Contributor

BertrandDechoux commented May 10, 2012

reference : https://github.com/nathanmarz/cascalog/wiki/JCascalog

Of course, it is "naturally more verbose" in java but I would wager the API could be improved thanks to :

  • removal of 'new' and class names
  • use of varargs
  • use of static import
  • use of method chaining
  • use of type checking
  • use of IDE completion

First attempt was to transform

Api.execute(
  new StdoutTap(),
  new Subquery(new Fields("?person", "?age", "?double-age"),
    new Predicate(Playground.AGE, new Fields("?person", "?age")),
    new Predicate(new Multiply(), new Fields("?age", 2), new Fields("?double-age"))
));

into

query("?person", "?age", "?double-age")
  .bind(Playground.AGE).to("?person", 25)
  .mult("?age", 2).to("?double-age")
  .writeTo(sdout());

And this can been see at the moment here : https://github.com/BertrandDechoux/cascalog/blob/fluent-jcascalog/src/jvm/jcascalog/fluent/Demo.java

Feedback from Nathan : http://groups.google.com/group/cascalog-user/browse_thread/thread/231af69ac6c8afb4

Would be better this way :

query("?person", "?age", "?double-age")
  .pred(PLAYGROUND.AGE, "?person", "?age")
  .pred(new GT(), "?age", 2));

The 'writing' part should be outside the API. It makes sense.
But I don't understand the use case of using (Tap, Query, Tap, Query, ...).
We should discuss it later but right now the solution proposed by Nathan (same api as clojure) is fine, even though it would not be type safe.

Predicates should not be inside the API. Due to their amount, they polute the api.
But a help class can still be provided so that we can write count() instead of new Count().
The pred(...) method makes understand better that a query is composed by a collection of predicates.

query("?person", "?age", "?double-age")
  .pred(PLAYGROUND.AGE, "?person", "?age")
  .pred(gt(), "?age", 2));

Of course, names could be changed along the way : new GT(), gt(), greater(), greaterThan(), desc(), descending()...

@BertrandDechoux

This comment has been minimized.

Show comment Hide comment
@BertrandDechoux

BertrandDechoux May 14, 2012

Contributor

rough draft : https://github.com/BertrandDechoux/cascalog/blob/feature/fluent-jcascalog/test/jcascalog/fluent/Demo.java

The integration tests are passing.

Cleaning will be needed.

Contributor

BertrandDechoux commented May 14, 2012

rough draft : https://github.com/BertrandDechoux/cascalog/blob/feature/fluent-jcascalog/test/jcascalog/fluent/Demo.java

The integration tests are passing.

Cleaning will be needed.

@nathanmarz

This comment has been minimized.

Show comment Hide comment
@nathanmarz

nathanmarz May 15, 2012

Owner

@BertrandDechoux This is cool stuff. Would like to get this merged into trunk... can you open up a pull request for this so we can iterate on the specifics?

Owner

nathanmarz commented May 15, 2012

@BertrandDechoux This is cool stuff. Would like to get this merged into trunk... can you open up a pull request for this so we can iterate on the specifics?

@sritchie

This comment has been minimized.

Show comment Hide comment
@sritchie

sritchie Aug 7, 2013

Collaborator

@BertrandDechoux, are you still interested in this?

Collaborator

sritchie commented Aug 7, 2013

@BertrandDechoux, are you still interested in this?

@BertrandDechoux

This comment has been minimized.

Show comment Hide comment
@BertrandDechoux

BertrandDechoux Sep 4, 2013

Contributor

The decision of @nathanmarz was in favor of a simpler version.

The discussion is here #81, it seems like I did not link back.

If nobody is interested, we might as well close this issue.

Contributor

BertrandDechoux commented Sep 4, 2013

The decision of @nathanmarz was in favor of a simpler version.

The discussion is here #81, it seems like I did not link back.

If nobody is interested, we might as well close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment