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

Add a Lexer/Parser for mathematical expressions. (Input Processor + Separate Action & Condition) #183

Open
wants to merge 12 commits into
base: 8.x-3.x
Choose a base branch
from

Conversation

fubhy
Copy link
Collaborator

@fubhy fubhy commented Apr 19, 2015

Fixing implementation of the Shunting-yard algorithm.

@fubhy
Copy link
Collaborator Author

fubhy commented Apr 19, 2015

This is not yet finished. It is lacking test coverage and the usage of plain arrays for the output and for the stack in the Shunting-yard algorithm is going to be subject to change (I want to use \SqlQueue and \SqlStack there... Same for the initial tokenizing).

Note: This implementation seems to be extremely effective due to the compiled regex. I tried running a few (even complex) mathematical expressions and they correctly evaluated within a matter of microseconds. Yai

@fubhy
Copy link
Collaborator Author

fubhy commented Apr 19, 2015

Oh and it obviously lacks Drupal / Rules integration (still misses variable support, currently only handles constants). I also want to register the lexer and the parser as separate services. Not sure yet how I want to handle the registration of the default methods. Probably through a list of method calls in the services.yml.

use Drupal\rules\Math\Token\NumberToken;
use Drupal\rules\Math\Token\OperatorTokenInterface;

class Calculator {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a docblock

@fubhy fubhy changed the title Initial commit for Math Lexer & Parser. Add a Lexer/Parser for mathematical expressions. (Input Processor + Separate Action & Condition) Apr 19, 2015
*
* @return $this
*/
public function addFunction($name, callable $function, $arguments = 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you figure out the amount of arguments using reflection?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would like to avoid reflection due to performance concerns. Do you think it is viable to do this on runtime? Or are you saying that, once I move this to some other place, I would do this during the building of the container?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point. Yeah I kinda assumed that you cache that object

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, I will... Eventually (once I am registering as a service). So I will look at that again once that's done. That huge constructor with all the registering is going to go away in that process of course (that's just for now so I can work with it and test it).

Copy link
Owner

Choose a reason for hiding this comment

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

Makes sense. The variable should be number_of_arguments or so though, as it's not the actual function arguments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once we retrieve that with reflection that argument would be gone anyways :P.

@fubhy
Copy link
Collaborator Author

fubhy commented Aug 31, 2015

I just made this a separate PHP library on GitHub: https://github.com/fubhy/math-php

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.

4 participants