Store expressions in array, then use in a loop?

[OsakaWebbie]

I build an HTML table based on data from a database, and depending on various user options, which columns are included changes quite a bit. I used to repeat a complex conditional structure three times: for an array of the class names and a parameter used to set up jQuery code, for the column headers, and for the data. But I'm tired of that mess, so I added the header text to the array, and thought I would do the same thing for the data. But of course the data is dynamic...

The following code is simplified, but imagine a lot more than these examples, intertwined in a labyrinth of if statements based on user selections. The array has values like this:

$cols[] = array("ddate",1,"<th class=\"ddate\">"._("Date")."</th>");
$cols[] = array("amount-for-csv",1,"<th class=\"amount-for-csv\" style=\"display:none\">"._("Amount")."</th>");
$cols[] = array("amount-for-display",0,"<th class=\"amount-for-display\">"._("Amount")."</th>");

And after I display the headers I loop through my data and do this:

echo "<td class=\"ddate\">".$row->DonationDate."</td>\n";
echo "<td class=\"amount-for-csv\" style=\"display:none\">".number_format($row->Amount,$_SESSION['currency_decimals'],".","")."</td>\n";
echo "<td class=\"amount-for-display\"><span style=\"display:none">".sprintf("%012s",$row->Amount)."</span>".$_SESSION['currency_mark']." ".number_format($row->Amount,$_SESSION['currency_decimals'])."</td>\n";

The first way I thought of to combine them was something like this (slapped together for this example - untested):

$cols[] = array("ddate",1,"<th class=\"ddate\">"._("Date")."</th>","<td class=\"ddate\">".$row->DonationDate."</td>");
$cols[] = array("amount-for-csv",1,"<th class=\"amount-for-csv\" style=\"display:none\">"._("Amount")."</th>","<td class=\"amount-for-csv\" style=\"display:none\">".number_format($row->Amount,$_SESSION['currency_decimals'],".","")."</td>");
$cols[] = array("amount-for-display",0,"<th class=\"amount-for-display\">"._("Amount")."</th>","<td class=\"amount-for-display\"><span style=\"display:none">".sprintf("%012s",$row->Amount)."</span>".$_SESSION['currency_mark']." ".number_format($row->Amount,$_SESSION['currency_decimals'])."</td>");

But then I'd have to run eval("echo...") for every cell in my <tbody>! I'm sure that would be horribly slow, and I would have to carefully check all string values for hackware. The other idea I had was something like this:

foreach ($cols as $col) {
  switch ($col[0]) {
  case "ddate":
    echo "<td class=\"ddate\">".$row->DonationDate."</td>\n";
    break;
  case "amount-for-csv":
    echo "<td class=\"amount-for-csv\" style=\"display:none\">".number_format($row->Amount,$_SESSION['currency_decimals'],".","")."</td>\n";
    break;
...etc...

It's a safer solution, but I don't know if it's much faster, as it would still have to run through a big switch (16 cases, only some of will be in any given table) for every cell.

Thoughts?

 

[jpadie]

why not put the cols array in a function?

function getCols($item, $override = false){
 global $cols;
 if ( empty($cols) || $override):
$cols = array(); //zero the array;
$cols[] = array("ddate",1,"<th class=\"ddate\">"._("Date")."</th>");
$cols[] = array("amount-for-csv",1,"<th class=\"amount-for-csv\" style=\"display:none\">"._("Amount")."</th>");
$cols[] = array("amount-for-display",0,"<th class=\"amount-for-display\">"._("Amount")."</th>"); 
endif;
  return isset( $cols[$item] ) ? $cols[$item] : '';
}

so just force an override (and thus a reinstantiation) when you think the dynamic data may have changed.

but personally I'd change the structure altogether and create a getter function that contains a struct type in the array (or even better, do it as a class)

function get($item) {
 static $array = array(
'Date'=> array('class' => 'ddate', 'style'=>''), 
'amount-for-csv' => array('class' => 'amount-for-csv', 'style'=>array('display'=>'none')));
  if (isset($array[$item]):
     $style = is_array($array[$item]['style']) ? implode (';',$array[$item]['style']) : $array[$item]['style'];
     return "<th class=\"{$array[$item]['class']}\" style=\"{$style}\">" . _($item) . "</th>";
 else:
  return '';
 endif;
}

 

[vacunita]

I'd probably just build the table cells as needed with a function:

function buildRow($rowVal,$class=null,$style=null,$celltype='td', $output=null)
{
  $class_str='';
  $style_str = '';
  $row_str = '';
  if(!isset($rowVal)
  {
    return false;
  }
  if($class!=null)
  {
   $class_str = 'class="' . $class . '"';
  }
  if($style!=null)
  {
    $style_str = 'style="' . $style . '"';
  }

$row_str = "<" . $celltype . " " . $class_str  . " " . $style_str . ">" . $rowVal . "</" . $celltype . ">";
 if(!$output)
 {
   return $row_str;
 }

 echo $row_str;
 return true;
}

Usage:

buildRow((number_format($row->Amount,$_SESSION['currency_decimals'],".",""),"ddate","display:none;","td",false);

You could of course modify it to use an array with the predefined classes and styles instead of passing them directly. This would shorten the usage considerably.

$stylings['ddate']['class']="ddate";
$stylings['ddate']['style']="display=none;"
$stylings['otherobj']['class']="other";
...

function ($rowVal,$valType,$cellType,$output)
{
...
}

----------------------------------
Phil AKA Vacunita
----------------------------------
Ignorance is not necessarily Bliss, case in point:
Unknown has caused an Unknown Error on Unknown and must be shutdown to prevent damage to Unknown.

Web & Tech

 

[OsakaWebbie]

Thanks to both of your for quick replies and detailed example code - you guys are great! But from your responses it seems that I oversimplified the code in my post to the point where you might not have caught my main question. Your suggestions for improving the structure of my code in various ways are good, but the key issue is not addressed.

...when you think the dynamic data may have changed.

The dynamic data (the part I get from the database, which you didn't have in your code examples) will change for every row in my DB query. So your first example would require recreating the entire array (including the labyrinth of conditionals I did not include in my simple code snippet) over and over, even though 3/4 of the elements will not change. I haven't tried to implement it, but I suspect it doesn't provide much speed gain over a switch statement for each cell. Your second example is elegant, but loses the ordered characteristic of the array - I don't need to access the information by name, but only walk through the columns in order over and over, so a numerically keyed array is more appropriate.

vacunita: Your code also makes things more tidy and readable, but doesn't account for the fact that I don't know which columns will be included until runtime, when the user chooses what type of table(s) they want. So I would have to either create your function dynamically (which would once again require eval, albeit once per row instead of once per cell) or include the labyrinth of conditionals inside the function. Since my objective is to only have one labyrinth instead of two (because maintenance is a nightmare!), that doesn't help.

For now, I decided to try implementing the switch/case to see just how slow it really is. I apparently created some other bugs (a Javascript library is protesting that my table structure doesn't fit what it is expecting, and there are other oddities when certain user options are selected), but the basic building of the table works - although I can't do a controlled performance benchmark on a hosted server, I'm pleasantly surprised that it doesn't seem annoyingly slow.

 

[jpadie]

if you post the full code snip i'm happy to take a look at whether there are better structures.

but evaluating conditionals is very straightforward and very very fast for processing languages; even interpreted ones like php. and a switch is just a simple form of conditional.

 

[OsakaWebbie]

The full code snip would be hundreds of lines - it would be hard to break it apart. But thanks for the encouragement about switch - perhaps I was overconcerned.

I intend to rewrite this whole application from the ground up in OOP, probably an MVC framework. (In fact, you and I had a conversion about OOP in March when you helped me with a gettext question - you are clearly good at it, but I'm still just learning.) Until that time, I have a considerable list of things I need to do to clean up the current procedural code for the sake of the users (including myself), and time is always too short...

I think I'll leave the code the way it is (with the overworked switch statement), fix the new bugs I created, and move on. But thanks for helping me brainstorm and showing me some techniques I can use later.