CODEWITHSUNDEEP
phpoptimization
Janaka Bandara
Janaka Bandara

Reputation: 72

Seeking advice to optimize the following code chunk. (I need to avoid the IF ELSES)

I have a JSON file with around 30 sub sections. Each subsection will be different. I wanted to convert the data inside it into a format i wanted. The code works fine. But i feel its not optimized 100%.

Client1Insurance, Client2Insurance, ClientFInsurance, FamilyInsurance, Client1Pension, Client2Pension, ClientFPension, FamilyPension.

Above is an example of how this JSON would look like. All above are arrays which have sub arrays inside them. There are around 30 arrays like this. 

foreach ($json as $item) {
 if (strpos($crmMapKey, "Insurance")) {
     $returnArray[] = $this->handleInsurance($item);
 } elseif (strpos($crmMapKey, "Pension")) {
    $returnArray[] = $this->handlePension($item);
 } ... continues the comparison till the json ends
}

I need a way to avoid this long if else comparions which I am not proud of. Will someone be able to suggest a better way to do this?

Thanks.

Upvotes: 0

Views: 49

Answers (2)

Jim Grant
Jim Grant

Reputation: 1148

Another way would be to call variable based functions.

foreach ($json as $item) {
    $returnArray[] = $crmMapKey($item);
}

function Client1Insurance($item) {
    // Do something in here.
    return $array;
}

function Client2Insurance($item) {
    // Do something in here.
    return $array;
}

Much more elegant way of doing things I feel.

Upvotes: 0

AbraCadaver
AbraCadaver

Reputation: 78984

If they are named the same as you show in your code, something containing Insurance will call handleInsurance, etc. then just get the term and use it in the method call:

preg_match('/Insurance|Pension/', $crmMapKey, $match);
$returnArray[] = $this->{'handle'.$match[0]}($item);

If not then you can use a lookup array:

$lookup = ['Insurance' => 'doSomething', 'Pension' => 'doAnotherThing'];
preg_match('/Insurance|Pension/', $crmMapKey, $match);
$returnArray[] = $this->{'handle'.$lookup[$match[0]]}($item);

Or use the keys in the pattern so you only have to modify the array:

preg_match('/'.implode('|', array_keys($lookup)).'/', $crmMapKey, $match);

The switch I mentioned in a comment might not be the best but works:

switch(true) {
    case strpos($crmMapKey, "Insurance") !== false;
        $returnArray[] = $this->handleInsurance($item);
        break;

    case strpos($crmMapKey, "Pension") !== false;
        $returnArray[] = $this->handlePension($item);
        break;

    //etc...
}

Upvotes: 1

Related Questions