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

fix: Execute nested functions #31

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

adlerfaulkner
Copy link
Contributor

Fixes #30

With this change, there are now three places where the code repeats a nearly identical block to execute a function:

const definition = functionHelper.findDefinition(data, functionValue.predicateObjectMap, prefixes);
const parameters = functionHelper.findParameters(data, functionValue.predicateObjectMap, prefixes);
const calcParameters = await helper.calculateParams(Parser, parameters, index, options, data, prefixes);
const result = await functionHelper.executeFunction(definition, calcParameters, options);

When I have some time I can start refactoring that stuff, unless someone is already working in that direction.
Some other work I'd love to see & help with in this repo is:

  • Conversion to typescript
  • More adherence to linting, I'm seeing tons of linter errors

@ThibaultGerrier
Copy link
Collaborator

Hey there, really cool stuff, seems to work well!

Tried it out with some yarrrml mappings with nested functions and it performs great:) Seems like the changes necessary to support this in the end were rather simple.

Also means that yarrrml conditions (https://rml.io/yarrrml/spec/#conditions) now work as well. Although the extra function yarrrml adds to the mapping would need to be added manually when using that feature. Something like the following seems to work:

      'http://example.com/idlab/function/trueCondition': (args) => {
        if(args['http://example.com/idlab/function/strBoolean']) {
          return args['http://example.com/idlab/function/str'];
        }
      },

Regarding this project's code quality, yeah it's not great. Not sure how feasible a conversion to typescript would be, adding type annotations would be one thing but improving the code quality in general another alltogether. At that point a complete rewrite would probably make more sense.
Besides the occasional bugfixes this repo isn't really worked on.

I'll merge this PR and publish it with a new minor version to npm.

@ThibaultGerrier ThibaultGerrier merged commit c4bc395 into semantifyit:master Jul 6, 2022
@adlerfaulkner
Copy link
Contributor Author

adlerfaulkner commented Jul 6, 2022

Awesome, thanks!

I may start working on the complete rewrite soon then. Will keep you updated.

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.

Nested functions do not get executed
2 participants