Alternative to using foreach twice in this logic

Question

Maybe I am looking past something, early morning.

I have an array of 'prescriptions' that I get from my database and I store it in a public variable in my class. A quick example of what that array looks like after the SQL call...

$prescriptions = [
    0 => [
        'companyName' => 'FoorBar Inc.',
        'prescription' => 'SomePrescription',
        'cost' => '$9.98',
    ],
    1 => [
        'companyName' => 'FoorBar Inc.',
        'prescription' => 'SomeOtherPrescription',
        'cost' => '$15.98',
    ],
    2 => [
        'companyName' => 'Labs Inc.',
        'prescription' => 'SomePrescription',
        'cost' => '$12.38',
    ],
    3 => [
        'companyName' => 'Lorem Ipsum Inc.',
        'prescription' => 'SomePrescription',
        'cost' => '$100.98',
    ],
    4 => [
        'companyName' => 'Lorem Ipsum Inc.',
        'prescription' => 'SomePrescription',
        'cost' => '$53.08',
    ],
];

Now what I am looking to do is group these prescriptions by company but still persist all data attached to the prescription, so I have the following logic:

public function groupByCompany()
{
    $clinicArray = [];

    foreach ($this->prescriptions as $prescription) {
        $clinicArray[$prescription["companyName"]] = [];
    }

    foreach ($this->prescriptions as $prescription) {
        $clinicArray[$prescription["companyName"]][] = $prescription;
    }

    return $clinicArray;
}

This works just fine, I get my desired result. I just feel like there is a better way to do this and maybe without running a foreach loop twice. Any suggestions?

Thank You Everyone.

I don't see anything wrong with putting both lines in the first loop. — Bluebaron, Aug 11 '17 at 13:08
`Any suggestions` .I'd start with "don't fix until it's broken". You are trying to optimize thing you are not even sure is not-optimal. Does it really makes sense in that order? — Marcin Orlowski, Aug 11 '17 at 13:08
@MarcinOrlowski I guess part of refactoring code is "fix if it can be better even if it's not broken". Is it not so? — Tom, Aug 11 '17 at 13:14
Sure, but first you need to know that it is bad, ineffective or faulty in that way or another. Having nested foreach does not automatically make your code such — Marcin Orlowski, Aug 11 '17 at 13:15

score 2 · Accepted Answer · edited Aug 11 '17 at 13:47

2

With one foreach it is:

public function groupByCompany()
{
    $clinicArray = [];

    foreach ($this->prescriptions as $prescription) {
        $companyName = $prescription["companyName"];

        if (!isset($clinicArray[$companyName])) {
            $clinicArray[$companyName] = [];
        }

        $clinicArray[$companyName][] = $prescription;

    }

    return $clinicArray;
}

edited Aug 11 '17 at 13:47

localheinz

9,179
2
33
44

answered Aug 11 '17 at 13:13

u_mulder

54,101
5
48
64

3

Using this awful foreach/endforeach syntax should be punishable... ;( – Marcin Orlowski Aug 11 '17 at 13:16
Beautiful, I knew I was just moving slow this AM, thanks! Will accept this as answer when I can ( 5 min wait ). – Taylor Foster Aug 11 '17 at 13:17
Great. Thank you. – Marcin Orlowski Aug 11 '17 at 13:26
@localheinz This is called `attempt to reply`. If you don't like some codestyle or function usage - write your own answer, __please__. – u_mulder Aug 11 '17 at 13:26
Not sure what your problem is, @u_mulder, I even upvoted your answer originally. If you don't like the features this platform provides, maybe find a new platform. – localheinz Aug 11 '17 at 13:30
Okay I can understand that some `foreach/endforeach` not please you, but what is the purpose of replacing `isset`? Because you like `array_key_exists` more than `isset`? – u_mulder Aug 11 '17 at 13:31
And same to you - if you don't like an answer, maybe you write your own? – u_mulder Aug 11 '17 at 13:32
See https://stackoverflow.com/questions/700227/whats-quicker-and-better-to-determine-if-an-array-key-exists-in-php. – localheinz Aug 11 '17 at 13:48

score 0 · Answer 2 · answered Aug 11 '17 at 13:22

There are many ways to change the logic in your code to group the loops.

The most common in PHP is to check if the item of the array already exists using the isset() function:

foreach ($this->prescriptions as $prescription) :

    if (!isset($clinicArray[$prescription["companyName"]])) {
        $clinicArray[$prescription["companyName"]] = [];
    }
    $clinicArray[$prescription["companyName"]][] = $prescription;

endforeach;

Alternative to using foreach twice in this logic

2 Answers2