3
\$\begingroup\$

I have an array with the following structure

Array 
( 
    [0] => Array 
        ( 
            [area] => Area ABC 
            [activity] => Array 
                ( 
                    [0] => Activity 1
                    [1] => Activity 2
                ) 
        ) 
) 

I'm trying to render this array in the following way

Area ABC

  1. Activity 1
  2. Activity 2

And am currently using the following code to achieve this

$status = 'closed';
foreach($suggestions as $suggestion)
    {
    if($currentarea != $suggestion['area'])
        {
        echo "<h3>";
        echo $suggestion['area'];
        echo "</h3>";
        if($status == 'closed')
            {
            echo "<ol>";
            $status = 'open';
            }
        }
    foreach($suggestion['activity'] as $activity)
        {
            echo "<li>";
            echo $activity;
            echo "</li>";
        }
    if($status == 'open')
        {
            echo "</ol>";
            $status = 'closed';
        }
    $currentarea = $suggestion['area'];
    }

Is this the best way of doing it, or is there a better way, I will have more areas and activities, but the structure stays the same.

\$\endgroup\$
3
  • \$\begingroup\$ I guess you're intializing status before the first loop, am I right ? Could you put that in your code, just so there is no confusion possible ? \$\endgroup\$
    – Loufylouf
    Jul 3, 2015 at 19:22
  • \$\begingroup\$ yes, that is correct, updated. Sorry for the confusion \$\endgroup\$
    – TDsouza
    Jul 3, 2015 at 19:34
  • \$\begingroup\$ Your code seems more complicated than it needs to be for this simple example. Please elaborate on your example data to include more areas and activities. \$\endgroup\$ Jul 3, 2015 at 19:49

2 Answers 2

3
\$\begingroup\$

This won't work if each suggestion does not start a new area.

foreach($suggestions as $suggestion)
    {
    if($currentarea != $suggestion['area'])
        {
        echo "<h3>";
        echo $suggestion['area'];
        echo "</h3>";
        if($status == 'closed')
            {
            echo "<ol>";
            $status = 'open';
            }
        }
    foreach($suggestion['activity'] as $activity)
        {
            echo "<li>";
            echo $activity;
            echo "</li>";
        }
    if($status == 'open')
            {
            echo "</ol>";
            $status = 'closed';
            }
    $currentarea = $suggestion['area'];
    }

You close on every iteration that it's open and add list items on every iteration, but you only open if the area changes.

foreach($suggestions as $suggestion)
{
    echo '<h3>';
    echo $suggestion['area'];
    echo "</h3>\n";
    echo "<ol>\n";

    foreach($suggestion['activity'] as $activity)
    {
        echo '  <li>';
        echo $activity;
        echo "</li>\n";
    }

    echo "</ol>\n";
}

This code is simpler and will work more reliably.

This is the wrong place to discuss how to write new code. But if you want to get the previous concept working, you'll want to open and close inside the if that checks if the area has changed. Or pull off the first element of the list so that you can treat it differently. Or make a new list where there's only one suggestion per area, combining multiple suggestions with the same area into one.

\$\endgroup\$
1
  • \$\begingroup\$ Each activity will always have an area attached to it, but yes, looking at your code I do feel I was over engineering a bit. I was working with different types of formats for the initial array, and in the process this code evolved (not to it's best in this case I guess). Thanks \$\endgroup\$
    – TDsouza
    Jul 3, 2015 at 20:15
3
\$\begingroup\$

I'll add to @mdfst13's answer that you should use concatenation when you want to display your strings. I get the idea that you want to separate things, but really, 3 lines to display a list item ?

One article I read gave a good example about that, with the ternary operator. Let's say you have to display some thing, and at a specific location, you could have the singular or plural form of a name (like "one knife" or "x knives"), depending on the value of a variable. Are you gonna really spend 6 lines of code (in the case you don't over-use newlines) just for that (1 for the first part of the sentence, 4 for the if/else, 1 to resume the sentence) ? Is that condition what is important in your code ?

Having a well-spaced code is important, but sometimes concision is also important. You just have to find the right balance.

\$\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.