Skip to content
This repository was archived by the owner on Jul 6, 2023. It is now read-only.

Conversation

@praveenag
Copy link
Contributor

neo4j> :param firstName => "Bruce"
neo4j> :param lastName "Wayne"
neo4j> :param alias: "Batman"
neo4j> :params
:param alias          => "Batman"
:param firstName      => "Bruce"
:param lastName       => "Wayne"
:param thisAssignment => "Works"
neo4j> :param hello => 23
neo4j> :param goodbye => "23"
neo4j> RETURN $hello, $goodbye;
+-------------------+
| $hello | $goodbye |
+-------------------+
| 23     | "23"     |
+-------------------+

1 row available after 11 ms, consumed after another 1 ms
neo4j> :params
:param alias          => "Batman"
:param firstName      => "Bruce"
:param goodbye        => "23"
:param hello          => 23
:param lastName       => "Wayne"
:param thisAssignment => "Works"

@praveenag praveenag requested a review from lutovich April 5, 2018 12:14
Copy link
Contributor

@lutovich lutovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@praveenag review completed. Made a comment about AbstractMap.SimpleEntry.

Couple things to think about (maybe already planned for future PRs?):

  • how would user define a temporal or spatial param?
  • will listing of temporal and spatial params include type so that they are visually different form strings?

// Final space to catch newline
protected static final Pattern cmdNamePattern = Pattern.compile("^\\s*(?<name>[^\\s]+)\\b(?<args>.*)\\s*$");
protected final Map<String, Object> queryParams = new HashMap<>();
protected final Map<String, AbstractMap.SimpleEntry<String, Object>> queryParams = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to create a simple class with String and Object fields instead of using very verbose AbstractMap.SimpleEntry<String, Object> everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd create temporal and spatial parameters using functions similar to the usage in queries ie: point({x:1, y:2})
About printing Spatial and Temporal types we based on @apcj excellent direction decided to use toString. I believe he'll be taking this discussion forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very much against using toString(). Java driver provides no explicit guarantees on the toString() format. Cypher shell has it's own requirements on how parameters should be formatted and thus should take care of the formatting. Please see the following two links for reasons why toString() is not the best choice: one & two

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I'm with you on that. But thats the interim decision.

@praveenag praveenag merged commit 76b5c64 into neo4j:master Apr 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants