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

Support for getting values from maps #70

Open
romangithub1024 opened this issue Aug 12, 2017 · 3 comments
Open

Support for getting values from maps #70

romangithub1024 opened this issue Aug 12, 2017 · 3 comments

Comments

@romangithub1024
Copy link

So I've been going around in circles, trying to make Knetic work with more complex expressions where you have to chain multiple calls.

My initial attempt was to use:

  1. "User(id).Name" - this doesn't work because such expressions can't be parsed. Ok, separate bug.
  2. After I changed code to use a field instead of a method, I was able to use "User.Name" directly, but it didn't get me very far
  3. My very next thing was to try "User.Attributes['attr_name']" - it doesn't work because there is no support for maps. Docs are clear on this, but it's a pretty serious limitation for what I'm trying to accomplish.
  4. I was thinking about a custom function, like "get(User.Attributes, 'attr_name')". But I started using Knect because of the short syntax, and this would be a big step back.

I guess my question is - given how accessors are implemented, would it make sense to add basic support for maps?

E.g. you could probably support syntax like "User.Attributes.attr_name" (not ideal, but better than nothing) by inserting a code here which uses reflection to check if it's a map (instead of a struct), and get the corresponding value:

if coreValue.Kind() != reflect.Struct {

Do you see any drawbacks to this approach?

I'm trying to figure how I can move forward with Knect instead of starting to write my own expression parser...

@romangithub1024
Copy link
Author

romangithub1024@fe2a164

@Knetic
Copy link
Owner

Knetic commented Aug 15, 2017

That approach is probably the best to get you unstuck for now, yeah. There's no technical drawback to it - I hadn't added map support yet because I'd preferred to have map keys be specified by braces (like User['name'], but those are already used for specifying full parameter names. I hadn't put too much work into it, though, and it might be easier than i was assuming.

Either way, the nuts and bolts of your implementation is pretty close to what I would have done to support maps!

Chaining the return value from multiple accessor function calls is on the top of my list for working on the library.

@romangithub1024
Copy link
Author

got it, thanks!

echlebek added a commit to echlebek/govaluate that referenced this issue Nov 22, 2017
Support nested Parameters by adding a type assertion to the query
loop in evaluationStage.go. Remove the check for unexported
accessors in the parser, and replace with a check for unexported
field access in evaluationStage.

This allows nesting maps in an arbitrary fashion with the
MapParameters type, as shown by the unit test.

Fixes Knetic#70
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

No branches or pull requests

2 participants