3
\$\begingroup\$

I was browsing Stack Overflow recently and saw a PHP question about rearranging an array of arrays for printing. The initial data structure looked like this:

$array1 = array(
    array("type01",1995),
    array("type03",1996),
    array("type01",1995),
    array("type09",1998),
    array("type09",1995),
    array("type02",1997),
    array("type01",1995),
    array("type02",1999),
    array("type09",1995)
);

For printing, the number of occurrences of each pairing--type and year--had to be counted and a table of frequencies made with type as columns and the years as the rows, i.e.

Year    type01    type02    type03    type09
1995    3         0         0         2
1996    0         0         1         0
1997    0         1         0         0
1998    0         0         0         1
1999    0         1         0         0

(the actual table formatting was not critical)

Someone else answered the question, and used the following code:

$sorted = array();
$types = array();
array_walk($array1, function($v) use (&$sorted, &$types){
   if( !isset($sorted[$v[1]]) )
       $sorted[$v[1]] = array();
   $sorted[$v[1]][] = $v[0];
   $types[] = $v[0];
});

// filter types
$types = array_unique($types);
sort($types);

// sort year
ksort($sorted);

echo '----' . implode(' ,', $types) . "\r\n";
foreach($sorted as $year => $data)
{    echo $year . ' - ';
    $count = array_count_values($data);

   foreach($types as $type)
   {  
      if( !isset($count[$type]))
      {
          echo '0,';
      } else {
          echo $count[$type].',';
      }
   }
   echo "\r\n";
}

I come from a Perl background, and I would use the following code to solve the problem:

$finalArray = array();
$years = array();

foreach ($array1 as $ix) {
    // $ix[0] is typeNN, $ix[1] is year.
    // index by type and then year.
    // the accumulator will count the number per type per year
    $finalArray[ $ix[0] ][ $ix[1] ]++;
    // keep an array of years
    $years[ $ix[1] ]++;
}

// sort arrays by key
ksort($finalArray);
ksort($years);

// print out the column headings
echo "Year\t" . implode("\t", array_keys($finalArray) ) . "\n";

// now add the data
foreach (array_keys($years) as $y) {
    echo $y;
    // for each type, check the frequency for the year
    // print '0' if the year does not exist
    foreach ($finalArray as $type => $year) {
        if (array_key_exists($y, $year)) {
            echo "\t" . $year[$y];
        } else {
            echo "\t0";
        }
    }
    echo "\n";
}

Granted, the two pieces of code format the table slightly differently, but is one approach "better"--more efficient, more in line with standard PHP coding practices--than the other?

I'm also aware that my code generates notices about undefined index/offsets because I'm used to Perl's autovivification of data structures. Is this something to worry about or not?

\$\endgroup\$
  • 2
    \$\begingroup\$ I wouldn't worry too much about notices, it won't effect the behaviour of your code, it is just there to help you find errors. To remove them you would likely need to do isset($val) first before using it. \$\endgroup\$ – bumperbox Sep 6 '14 at 7:50
2
\$\begingroup\$

In terms of the original code I find it a bit more complex than it needs to be. Once you get to the point where code will exist for a long time or be used by other people a cleaner/simpler solution with similar attributes is often a better bet.

Anyway, I created a solution which is very similar to your own but there are a few differences that might be worth highlighting.

$years = array();
$types = array();
$ytx   = array();

foreach ($array1 as $item)
{
  list($type,$year) = $item;

  $years[$year] = $year;
  $types[$type] = $type;

  if ( !isset($ytx[$year][$type]) )
    $ytx[$year][$type]=0;
  $ytx[$year][$type]++;
}

ksort($years);
ksort($types);

I'm big on simplification and variables are relatively cheap. So, above there are arrays for the years and types as well as new variable collecting the count of year/type instances.

For sorting purposes the year and type keys are set to the year or type while the (dummy) value used is the same which allows simple foreach use later.

echo "<pre>Year";
foreach ($types as $type)
  echo "\t$type";
echo "<br>\n";

foreach ($years as $year)
{
  echo "$year";
  foreach ($types as $type)
  {
    $val = 0;
    if ( isset($ytx[$year][$type]) )
      $val = $ytx[$year][$type];
    echo "\t$val";
  }
  echo "<br>\n";
}
echo "</pre>\n";

The above is also very similar. However, you'll notice I avoided the else statement by setting a variable to zero. There doesn't seem to be a need to be overly concerned for efficiency so, to me, avoiding the else is just a little bit cleaner -- the same reason I often avoid the brackets for single line if statements though some will not.

A few other general review notes:

  • Strive for meaningful variable names. I tend to us shorter names than many folks do these days (such as $ytx) but in the context of the code I find them meaningful.

  • I didn't find a need to use all kinds of array related functions. If we are walking through the array anyway things like array_count_values, implode, array_unique, array_key_exists and so on aren't necessarily adding much value. Someone maintaining the code in the future may not be very familiar with PHP's array functions.

In short, I'd prefer your solution to the original. However, if it was being created for inclusion on a project of mine there is no way I'd accept it with the undefined index notices.

Other than that, get into the habit of putting time into what I call simplification. The total project effort can often be significantly reduced with design choices and in terms of long term maintenance simpler code is faster to modify and less risky to work with. If you don't think about simplification during smaller efforts it is not going to be second nature when you get into larger projects.

Of course, your view of simplicity may not coincide with mine which is fine as long as you don't work for me. ;)

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