5
\$\begingroup\$

In this case I have an array that is parsed from a JSON file. Normally I use all the elements in the array, but for AJAX crawling I only show the parts the search engine asked for:

<?php $fitstyles = json_decode(file_get_contents(DATA_FILE), true); ?>
…
<?php
if ($base == "fits" || $base == "gallery") {
    /* For fragment handler, delete out the parts of the fits array that aren't needed. */
    $keep_fit = $fragments[1];
    $keep_style = $fragments[2];
    foreach ($fitstyles as $fit => $styles) {
        if ($fit != $keep_fit) {
            unset($fitstyles[$fit]);
            continue;
        }
        foreach ($styles as $style => $styleinfo) {
            if ($style != $keep_style) {
                unset($fitstyles[$fit][$style]);
                continue;
            }
        }
    }
} ?>

Here, $fragments looks like:

(
    [0] => fits
    [1] => straight
    [2] => utility
)

and $fitstyles takes the following form:

Array
(
    [straight] => Array
        (
            [utility] => Array
                ( … )
        )
    [anotherfit] => Array
        (
            [anotherstyle] => ( … )
        )
)

The approach appears to work; however, please give me feedback as to the effectiveness, efficiency and readability of this approach.

\$\endgroup\$
  • 1
    \$\begingroup\$ IMO it would be more clear if you inverted the logic and built the resulting array instead of modifying an existing one (you can just assign $fitstyles with the resulting array if you want to keep the changes localized). I don't know enough about PHP to comment on whether this has any impact on efficiency. \$\endgroup\$ – user786653 Nov 9 '11 at 17:05
  • \$\begingroup\$ @user786653 I'd agree with you if $fitstyles were being put together iteratively, but the entire array comes from a one-line call to json_decode. I've edited my question to include that. Is there some alternative approach to parsing the JSON that would allow me to cherry-pick a given key? \$\endgroup\$ – kojiro Nov 9 '11 at 17:24
  • 1
    \$\begingroup\$ I don't know, sorry, I was just commenting in general. Writing it the other way round seems more naturally (again this is only IMO) fits the problem statement and you're more likely to be able to extend and reuse the code for it (or find a built-in function to replace it). \$\endgroup\$ – user786653 Nov 9 '11 at 17:52
3
\$\begingroup\$

Your code is just fine, exactly how I would do it.

There isn't much to discuss really, it's kind of simple. It's as effective / efficient as it could be, and fairly readable.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.