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

can not pass an array of functions into higher order function like map #268

Closed
cuichenli opened this issue May 17, 2023 · 6 comments
Closed

Comments

@cuichenli
Copy link

Following is the code I tried:

(
  $returnTrue := function() {
      true
  };

  $returnFalse := function() {
      false
  };
  $flist := [$returnTrue, $returnFalse];

  $map($flist, function($ff) {
      $ff()
  })
)

It works correctly on https://try.jsonata.org/ as following:
image

But it won't work with JSONata4Java as following:
image

@wnm3
Copy link
Member

wnm3 commented May 17, 2023

This works. Note the parentheses after the function calls in $flist:
( $returnTrue := function() { true }; $returnFalse := function() { false }; $flist := [$returnTrue(), $returnFalse()]; $map($flist, function($v){$v} ))

This also works:
( $returnTrue := function() { true }; $returnFalse := function() { false }; $flist := [$returnTrue(), $returnFalse()]; $map($flist, function($ff){$ff} ))

If you agree, please close this issue.

@wnm3
Copy link
Member

wnm3 commented May 17, 2023

The problem is, without the parentheses, we interpret the $returnTrue to be a variable reference and not a function call.

@wnm3
Copy link
Member

wnm3 commented May 17, 2023

Another problem is the language we use defined a variable as the $ID without parentheses, so the reference to $returnTrue (or $returnFalse) in the flist array sees them as variables, and variable recall wants to retrieve a JsonNode, not a function. The way this is working in jsonata.org is the $flist is an array of functions (hence the need to provide the parentheses for $ff in the map's function).

I implemented a patch that executes a reference to a function declared as a variable (as in the flist declaration) but the problem is it attempts to execute the function at the time the variable is referenced (e.g., when creating the flist array). So with the patch you can execute:

( $returnTrue := function() { true }; $returnFalse := function() { false }; $flist := [$returnTrue, $returnFalse]; $map($flist, function($ff){$ff} ))

e.g,. without needing the parentheses for the $returnTrue, $returnFalse parameters.

But this isn't a very good solution IMHO.

You can try this by adding these lines in the ExpressionVisitor starting after line 3030:
JsonNode result = getVariable(varName);

        if (result == null) {
          // see if this is calling a declared function with no parameters
          DeclaredFunction declFct = getDeclaredFunction(varName);
          if (declFct != null) {
            ExprListContext exprListCtx = declFct.getExpressionList();
            result = visit(exprListCtx);
          }
        }

The reason I don't like this solution is it would only work for functions that have no parameters, and the existing code would work if you added parentheses when referencing the functions in flist.

Perhaps I need more details about the real use case to appreciate what you are trying to accomplish.

@cuichenli
Copy link
Author

Perhaps I need more details about the real use case to appreciate what you are trying to accomplish.

there is no actual use case for it - i report it mainly due to the inconsistency i noticed between jsonata4java and jsonata.org (along with #267 ). and i think the workaround is sufficient for my use case.

but that said, any chance we can update the readme so let the users know there is a gap between jsonata4java and jsonata? thanks!

@wnm3
Copy link
Member

wnm3 commented May 18, 2023

I've added below to the readme. Please close if this is acceptable (and #267 as well).
image

wnm3 added a commit that referenced this issue May 18, 2023
wnm3 added a commit that referenced this issue May 18, 2023
@cuichenli
Copy link
Author

thanks!

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