refactor: use wrapper function for invalidLitteral protection

comunica / sparqlee

⚙️ SPARQL expression evaluator library - Moved to @comunica/expression-evaluator

https://www.npmjs.com/package/sparqlee

14 stars 7 forks source link

refactor: use wrapper function for invalidLitteral protection #176

Closed jitsedesmet closed 1 year ago

jitsedesmet commented 1 year ago

This PR aims to fix #115 .

coveralls commented 1 year ago

Coverage: 100.0%. Remained the same when pulling c6635ddd216fc861e7e2fc41ed6941c4f7046fd3 on jitsedesmet:refactor/fix-invalid-literals into a375e04b1944d29957a051c8710399744c4a3010 on comunica:master.

rubensworks commented 1 year ago

I just have two questions on this:

Does this have any impact on performance? (if so, how much)
Do all spec tests (in comunica) still pass with this change?

jitsedesmet commented 1 year ago

I checked the Comunica test suite last week and everything still seemed to be working.

As for performance, I posted an explanation in the related issue https://github.com/comunica/sparqlee/issues/115#issuecomment-1518646087 . It boils down to having a single if statement before each function. I think compared to the work done in the overloadTree, this should have minimal impact.

jitsedesmet commented 1 year ago

I have tested again (to be sure), and can confirm the comunica tests still work (sorry, running these kill my laptop 60% of the time).