-1

So I was about to write something like following code

switch ($mode) {
    case 'all':
        foreach ($sortInfo as $info) $filter[] = array_merge([$info->maincat], $info->subcats);
        break;
    case 'sub':
        foreach ($sortInfo as $info) $filter[] = $info->subcats;
        break;
    default:
        foreach ($sortInfo as $info) $filter[] = [$info->maincat];
        break;
}

Then I told myself "hey, wouldn't it be more optimized if I wrapped the whole switch inside the for loop?"

foreach ($sortInfo as $info) {
    switch ($mode) {
        case 'all':
            $filter[] = array_merge([$info->maincat], $info->subcats);
            break;
        case 'sub':
            $filter[] = $info->subcats;
            break;
        default:
            $filter[] = [$info->maincat];
            break;
    }
}

But while it does technically save a bit (get it?) of filesize, the loop would confirm the same information in the switch statement for each iteration of the loop.

I figured there is no objective way to determine which is fastest because it depends on the length of $sortinfo, but since it's the first time I come across this dilemma, I'd like to know if there's a preferred method, or what's your take on it.

Gummy
  • 11
  • 5
  • One thing is that `array_unshift` returns an int and not an array like the other options. – Nigel Ren May 10 '22 at 19:45
  • First one is more readable imho (because anything that does not depend on the loop should be outside the loop, namely the switch). And unless you have arrays with billions of entries at which point the runtime might matter it mostly is not sensible to reason about premature optimizations like this. – luk2302 May 10 '22 at 19:48
  • @NigelRen whoops, you're right, you just saved me about 7mins of confusion – Gummy May 10 '22 at 19:49
  • Well the "objective way" to test it is by running a set of fixed test data through both versions e.g. 10K times (enough to see a difference). Chances are you only need to run it once though, and as such any performance difference should be minimal. I find the second one more readable. You could replace the `foreach` loops with `array_column()` if you wanted to save more bits. – Markus AO May 10 '22 at 19:50
  • @MarkusAO Indeed, very minimal, but most of my years of coding were done on potato laptops so I always search for more efficient and ask for the best habits when, like this case, I'm confronted with two new solutions. Also thanks for the `array_column()` proposition, but it doesn't apply to my case, I edited the question so that the types are clearer. – Gummy May 10 '22 at 20:07
  • Yeah we do what we can to make sure the potato doesn't lag. So `->maincat` is a string and `->subcats` is an array of strings, and in each case the result should be a _single array of arrays_? Shouldn't pose a problem, let me know and I'll update the answer. – Markus AO May 10 '22 at 20:18
  • Providing a [mcve] by offering sample input and your exact desired output would make your question more valuable and clear. Does the order of the elements in the output matter? Storing the output data row by row seems like it will have a different result than storing data column by column ...if it matters. – mickmackusa May 12 '22 at 05:49
  • @Gummy this also seems like a viable alternative that doesn't require a `switch` or `match` block: https://3v4l.org/cjnie I'd like to see a [mcve] before I post an answer though. – mickmackusa May 12 '22 at 06:29

3 Answers3

2

Since you will only be running this once, any performance difference is entirely negligible. You could benchmark with a fixed dataset if you had to run it thousands of times, though still I doubt that you'd see more than a +/- 10% difference. Choose whichever version you find more readable. If you want to save bits and are running PHP 8, you can do:

$cats = match($mode) {
    'all' => array_merge(
        array_column($sortInfo, 'maincat'), ...array_column($sortInfo, 'subcats')
    ),
    'sub' => array_merge(...array_column($sortInfo, 'subcats')),
    default => array_column($sortInfo, 'maincat')
};

Updated: Per OP's revision, maincat is a single scalar and subcats is an array of scalars. Since we want to have a 1-D array in all modes, we use the ... splat operator to "dish out" the subcategories into the array_merge, which gives us a "flat" array. Demo: 3v4l.org/NasiC

That's 227 bits vs. 338 bits in your "switch in foreach" vs. 346 bits in your "foreach in switch" or a 33% reduction. I find this approach very readable and free of avoidable verbosity.

If you're still runnning PHP 7, you can factor this approach into a switch:

switch ($mode) {
    case 'all': 
        $cats = array_merge(
            array_column($sortInfo, 'maincat'), ...array_column($sortInfo, 'subcats')
        );
    break;
    case 'sub': 
        $cats = array_merge(...array_column($sortInfo, 'subcats'));
        break;
    default: 
        $cats = array_column($sortInfo, 'maincat');
        break;
};

That's still only 299 bytes. :) N.B. match was one of my major reasons for upgrading early to PHP 8. Premium sugar for a number of dishes, eliminates a lot of boilerplate code.

Markus AO
  • 4,771
  • 2
  • 18
  • 29
  • Thanks! I am currently under PHP 7.4.29, but I'll take a look for sure – Gummy May 10 '22 at 19:59
  • @AbraCadaver when I see an opening to promote `match` I take it! – Markus AO May 10 '22 at 20:00
  • @Gummy you can also factor the `array_column` approach into a switch. Updated the post to match. – Markus AO May 10 '22 at 20:04
  • @Gummy I've updated the answer to reflect your revision. The subcats are now merged with the splat operator from the `array_column` call, resulting in a 1-D array in all cases. I assume this was your objective? See here: https://3v4l.org/NasiC#v8.1.5 – Markus AO May 10 '22 at 20:59
  • @MarkusAO OH RIGHT I FORGOT ABOUT UNPACKING! thanks, it's something I used on few occasions with javascript but I didn't know PHP had one. "splat" huh, Sounds appropriate :P – Gummy May 10 '22 at 23:41
  • @MarkusAO I just noticed you kept the comma after the first `array_merge`, I assume this is a mistake, but I can't seem to edit your answer (_Suggested edit queue is full_) just for that tiny detail. – Gummy May 11 '22 at 15:11
  • @Gummy ah yes you're right, I tested with the `match` and just did a quick rewrite to a `switch`, and I see there was a leftover comma there. Fixed! PHP manual refers to it as _`...` operator, also known as "splat" operator in other languages_, which I suppose is a reference to Ruby. JS calls it [spread operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax). Others again call it, literally, "ellipsis". – Markus AO May 11 '22 at 17:29
0

Okay, it looks like the asker is not going to respond to my request for sample data and expected result.

To minimize the number of iterated conditions, you can determine if the mode is "sub", and simply collect that column of data (without any convoluted handling).

Otherwise, it is known that "maincat" data will be required in the result. The only thing to determine is if "subcats" should be pushed into the result (PER ROW -- otherwise the "maincat" and "subcats" relationships will be destroyed).

This is direct and concise, but it doesn't enjoy optimal maintainability like a switch() or match() block would. If you don't expect to extend the functionality of the snippet, I'd use a condition before the loop and a condition within the loop.

Code: (no demo because no sample data was provided)

if ($mode === 'sub') {
    $filter = array_column($sortInfo, 'subcats');
} else {
    $filter = [];
    foreach ($sortInfo as $info) {
        $filter[] = [$info->maincat, ...($mode === 'all' ? $info->subcats : [])];
    }
}
mickmackusa
  • 43,625
  • 12
  • 83
  • 136
  • I'm sorry if I couldn't respond in time. I actually did start a fully complete revision of my question, but I was running out of time on my internship and have already accepted an answer that lead me to a solution. My desktop litterally has a file named `newanswer.txt`. I put it on the side until I was ready to invest time updating my question as you requested. I really like your solution too, I didn't think about using a ternary operator to minimise the cases, but it does make me wonder why you would downvote my question if you could propose a solution without it? – Gummy May 24 '22 at 14:24
  • Good questions have a [mcve]. This makes it 100% clear which answers are in/correct. We don't know if MarkusAO's answer is actually suitable -- if I am correctly comparing your code versus his code, then they will produce different data structures. The dv is on the quality/completeness of the post, not a dv on the poster. When the question is edited to be complete, I am very happy to remove the dv and possibly uv. 95% of my answers have a demo link to prove correctness -- I could not provide a demo link on this answer. – mickmackusa May 24 '22 at 21:07
  • Okay I see what you mean. I'll complete the minimal reproductible example this weekend. – Gummy May 25 '22 at 12:22
-1

Basicly, the code and outcome are the same, but i think the first one would be faster because it is more simple and straight-forward. The second one looks cleaner but it needs to switch-case and make decision everytime it loop and this would slow down the whole process.

Hey
  • 11
  • 3
  • The impact of a simple string match between three options in a `switch` would be negligible though. If I run a blank loop for 5M times, it takes me ~0.11 sec. If I have a three-case `switch` in it, it takes ~0.20 sec (and ~0.25 sec for a three-arm `match`). Five million times mind you. – Markus AO May 10 '22 at 20:13