CODEWITHSUNDEEP
phparraysreferencevariable-assignment
Hamster
Hamster

Reputation: 3132

Digging into an array by reference isn't working?

It's easier just to give a code sample:

private $ParseRuleMap = array();

public function __construct( $rules ) {
    foreach( $rules as $which=>$rule ) {
        $mapping = $rule->getMinimumMatchables();

        foreach( $mapping as $match ) {
            $rulelvl &= $this->ParseRuleMap;    // Fun begins here

             $len = strlen($match);
            for( $i=0; $i<$len; $i++ ) {
                if( !isset($rulelvl[ $match[$i] ]) ) {
                    $rulelvl[ $match[$i] ] = array();
                }
                $rulelvl &= $rulelvl[ $match[$i] ]; // Here too!
            }

            // ... other code here ...
        }
    }
}

I get the following error spamming frequently (for the above commented lines):

PHP Warning: Cannot use a scalar value as an array in parser.php on line 35

Am I misunderstanding how reference assignment works here? To (try) to be clear, $rulelvl is supposed to iterate down through a given line of $this->ParseRuleMap's sub-arrays via. reference-assignment.

Upvotes: 1

Views: 496

Answers (2)

hakre
hakre

Reputation: 197832

Another tip I place here as an answer even it is merely a comment.

You already know what you did wrong, however it might not be clear why. Sure you mistyped something, but just a short heads up:

1.) Your constructor is doing too much. Put what is done here into a function of it's own.

public function __construct($rules) {
    $this->processRules($rules);
}

private function processRules($rules) {
    foreach ($rules as $which => $rule) {
        ...
    }
}

This reduces the complexity of the constructor. Also later on you might already want to pass the correct object into the constructor so that you can remove that pre-processing out of the whole class anyway. But that is not necessary for now and might even never become necessary so just giving some outlook.

2.) The processing itself is much nested and complicated. Reduce complexity by dividing a large problem into smaller parts.

Naturally this is related to what you need, hopefully the following code gives some useful examples how you can reduce complexity by splitting across multiple functions:

private function processRules($rules) {
    foreach ($this->rulesGetMappingsMatches($rules) as $match) {
        $this->parseRuleMapMatch($this->parseRuleMap, $match);
    }
}

private function parseRuleMapMatch(&$parseRuleMap, $match) {
    $len = strlen($match);
    foreach(str_split($match) as $char) {
        isset($parseRuleMap[$char])) || $parseRuleMap[$char] = array();
        $parseRuleMap = &$parseRuleMap[$char];
    }
    ...
}

private function rulesGetMappingsMatches($rules) {
    $matches = array();
    foreach ($rules as $rule) {
        foreach ($rule->getMinimumMatchables() as $match) {
            $matches[] = $match;
        }
    }
    return $matches;
}

3.) Do not use references where you do not need them.

I have no clue why in your scenario you make use of references. To get a nicer variable name? Then it might be okay. For speed improvements? Then you should not do that unless you really know what you are doing because PHP is pretty good in optimizing for speed and memory. And often it's better to have a function return a value instead of passing by reference and modifying that value then. That also help with code-reusage and debugging.

private function processRules($rules) {
    foreach ($this->rulesGetMappingsMatches($rules) as $match) {
        $this->parseRuleMap = $this->parseRuleMapMatch($this->parseRuleMap, $match);
    }
}

private function parseRuleMapMatch($parseRuleMap, $match) {
    ...
    return $parseRuleMap;
}

4.) The most simple solutions are most often the solution.

Well, just exemplary:

public function __construct( $rules ) {
    $this->importRulesMapFromArray($rules);
}

Should be pretty self speaking. Divide and conquer. Also give good names. You will make less mistakes in writing code.

Upvotes: 1

Ivan Hušnjak
Ivan Hušnjak

Reputation: 3503

&= is a bitwise operator (bitwise "and" and assign) not the reference operator

Change your code as this:

$rulelvl = &$this->ParseRuleMap;    // note the = &

Upvotes: 4

Related Questions