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

Use '.@' at the end of the param, to pass effective path instead of the value #1

Merged
merged 8 commits into from
Jan 31, 2018
Merged

Use '.@' at the end of the param, to pass effective path instead of the value #1

merged 8 commits into from
Jan 31, 2018

Conversation

aszu80
Copy link

@aszu80 aszu80 commented Jan 28, 2018

This is something I was thinking of.

Such a path specification received as a param would make some things a lot easier.

@coveralls
Copy link

coveralls commented Jan 28, 2018

Pull Request Test Coverage Report for Build 10

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 89.637%

Totals Coverage Status
Change from base Build 3: 0.3%
Covered Lines: 115
Relevant Lines: 120

💛 - Coveralls

src/rule.js Outdated
@@ -57,7 +59,13 @@ const defaultEnabler = [() => true],
},
queryPath = (value, context) => (p) => {
if (isFunction(p)) return p()
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, this is good, my only doubt is that we are taking another symbol "@" as operation. Intention is to make path as less DSL filled as possible. What do you think of another approach, where we can add current path as a parameter to a function here? Not just p() but p(contextPath)?

Copy link
Owner

Choose a reason for hiding this comment

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

Function as parameter will have something extra in terms of input data, it might be context path and value also

Copy link
Author

@aszu80 aszu80 Jan 30, 2018

Choose a reason for hiding this comment

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

I did it somewhat more as a "function as decorator" than "function as parameter".

This way we have someting that is looking as functional thing (and for simplicity i decided to go with the creating function that has a special "tag value" assigned as own key).

Other implementations I've done was based on the getPathTo returning object that had "original parameter" and flags (which was effectively pretty much the same as this implementation, but had more lines and ifs) and the "hoc style" that has created new function that was "identifying" itself when it received special argument (and that was too magical, and there was a chance of side effects if someone passed function as a param and if that function had side effects).

The most "stylish" would be probably based on class, then we could check if the object is instance of some our tool class, and then we could use that information to get the path instead of data, but that would then be biggest in the LOC sense, I think.

I agree, this is not ellegant to just "attach" own key to the function, but I think it's a) small, b) easy, c) fast.

@komlev komlev merged commit c1b1895 into komlev:master Jan 31, 2018
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.

None yet

3 participants