Adding custom symbols doesn't work (internal symbol container issue)

[jkphl]

First of all thanks for this nice and thoughtful piece of software! I'm about trying to use it for a new library I'm working on and it looks like it could serve me well for a particular use case. However, it looks like I discovered an issue with adding custom symbols (tl;dr: it doesn't work at the moment ...). Consider this test scenario (namespaces & comments omitted for the sake of brevity):

class Test extends AbstractFunction
{
    protected $identifiers = ['test'];

    public function execute(array $arguments)
    {
        return 1;
    }
}

$stringCalc = new StringCalc();
$stringHelper  = $stringCalc->getContainer()->get('stringcalc_stringhelper');
$stringCalc->addSymbol(new Test($stringHelper));
echo $stringCalc->calculate('test()');

I would expect this to yield just 1. However, I get this exception (because the parser cannot find the test() function):

ChrisKonnertz\StringCalc\Exceptions\NotFoundException : Error: Detected unknown or invalid string identifier.

Changing this line to the following fixes the problem:

// $parser = new Parser($symbolContainer);
$parser = new Parser($this->symbolContainer);

It looks like $this->container->get('stringcalc_symbolcontainer') is different from $this->symbolContainer despite this assignment.

I can avoid this bug temporarily by modifying the source as shown but it would be great to have the issue fixed upstream asap as automatic build tests are failing as well. (By the way you could also add to the documentation the fact that you need to pass a StringHelper instance to the function object constructor — just missing this in the docs).

[chriskonnertz]

Hello,

By the way you could also add to the documentation the fact that you need to pass a StringHelper instance to the function object constructor

yep, the README.md is not correct when it describes how to add a new symbol. It ignores the StringHelper dependency.

I get this exception

I get the same exception when I run your code example.

It looks like $this->container->get('stringcalc_symbolcontainer') is different from $this->symbolContainer

If this is true then your solution is only a workaround, right? The acutal issue still exist. I will try to figure out why there are two instances of a class that is meant to be soem kind of singleton (at least that's what i remeber right now).

[jkphl]

@chriskonnertz Sounds about true — if there's a singleton mechanism involved, it doesn't seem to work. ;)
In any case, the proposed PR works and solves the issue for me and I'll stick to it (aka my fork) until you fixed the issue upstream.

[jkphl]

@chriskonnertz Besides: nice and useful piece of code — I'm "misusing" it for parsing CSS calc() functions including values with units attached ...

[chriskonnertz]

Sounds about true — if there's a singleton mechanism involved, it doesn't seem to work. ;)
In any case, the proposed PR works and solves the issue for me and I'll stick to it (aka my fork) until you fixed the issue upstream.

I will merge your solution. It makes sense to use the $this->symbolContainer property instead of retrieving the service again. The latter is useless and a unecessary overhead.

Besides: nice and useful piece of code, works

Thank you!

I'm "misusing" it

I do not think you are "misusing" it ;)