PHP/Html Table class, is this a good design?

[Hall of Famer]

This is a table class I created, the purpose of doing this is that the software I am working on has too many hard coded php tables and forms. It definitely needs some improvement, but I am guessing it is quite useful to construct simple tables, especially for tables used to show database results.

 

The class file:

<?php
class Table {
  public $tablename;
  public $columns = array();
  public $columnwidth;
  public $tablewidth;
  public $style;
  public $alignment;
  public $cells = array();
  public $background = array();
  public $content;
  
  public function __construct($tablename, $columns, $columnwidth = "", $tablewidth = "", $background = ""){
      // Create the table
      $this->tablename = $tablename;
      if(!is_array($columns)) die("Cannot create a table with only one column...");
      if(is_array($columnwidth) and count($columns) != count($columnwidth)) die("Specification of column width is incorrect, please report this to administrator.");
      $this->columns = $columns;
      if(is_array($columnwidth)){
         foreach($columnwidth as $width){
            $this->columnwidth[] = "width='{$width}'";
         }
      }
      elseif(!empty($columnwidth) and !is_array($columnwidth)) $this->columnwidth = " width='{$columnwidth}'";
      else $this->columnwidth = "";      
      $this->tablewidth = $tablewidth;
      $this->background = $background;
      $this->content = "<br>";
  }

  public function createtable($alignment = "", $style = "", $border = "yes"){
      // This method builds the interface of the table, it must be chained with showtable() or buildtable()
      $this->alignment = $alignment;
      $this->style = $style;
      $this->columns = $this->formattable($this->columns);
      
      // Construct the basic table structure
      $this->content .= "</br><table";
      $this->content .= (!empty($this->tablewidth))?" width='{$this->width}'":"";
      $this->content .= ($border == "yes")?" border='1'":"";
      $this->content .= (!empty($this->background))?" {$this->background[0]}='{$this->background[1]}'>":">";
      
      // Create the very first row of table
      $i = 0;
      $this->content .= "<tr>";
      foreach($this->columns as $column){
         $this->content .= "<td {$this->columnwidth[$i]}>{$column}</td>";
         $i++;
      }
      $this->content .= "</tr>";
      return $this;      
  }
  
  public function buildtable($cells, $alignment = "", $style = ""){
       // This method will build the body of our table, it must be chained with showtable() or endtable()
       $this->content .= "<tr>";       
       $this->alignment = (!empty($alignment))?$alignment:$this->alignment;
       $this->style = (!empty($style))?$style:$this->style;
       $this->cells = $this->formattable($cells);
       
       // Now it is time to construct a row of our table
       $i = 0;
       foreach($this->cells as $cell){
         $this->content .= "<td>{$cell}</td>";
         $i++;
       }
       $this->content .= "</tr>";
       return $this;      
  }
  
  private function formattable($text){
      // Thie method formats the content of tables with alignment or style, can only be called witin the table class
      if(!is_array($text)) die("Cannot format the table content.");
      if(!empty($this->style)){
         if(is_array($this->style) and count($this->style) != count($text)) die("Cannot specify the style of table columns...");
         for($i = 0; $i < count($text); $i++){
             $text[$i] = (is_array($this->style))?"<{$this->style[$i]}>{$text[$i]}</{$this->style[$i]}>":"<{$this->style}>{$text[$i]}</{$this->style}>";
         }
      }
      if(!empty($this->alignment)){
         if(is_array($this->alignment) and count($this->alignment) != count($text)) die("Cannot specify alignment of table columns...");
         for($i = 0; $i < count($text); $i++){
             $text[$i] = (is_array($this->alignment))?"<{$this->alignment[$i]}>{$text[$i]}</{$this->alignment[$i]}>":"<{$this->alignment}>{$text[$i]}</{$this->alignment}>";
         }
      }
      return $text;
  }
  
  public function showtable(){
      // This method allows the page to show an unfinished table, incase control blocks, loops or forms need to be used
      $content = $this->content;
      unset($this->content);
      return $content;
  }
  
  public function endtable(){
      // This method terminates the construction of a table
      $this->content .= "</table>";
      return $this->content;
  }
} 
?> 

 

And here is an example, the output is also shown below:

<?php

include("classes/class_tables.php");

$content = "";

$table = new Table("Mytable", array("Column1", "Column2", "Column3", "Column4"), 100, 400, array("bgcolor", "#FFA500"));
$content .= $table->createtable("left")->showtable();
$content .= $table->buildtable(array("r1c1","r1c2","r1c3","r1c4"), "center", "strong")->showtable();
$content .= $table->buildtable(array("r2c1", "r2c2", "r2c3", "r2c4"), "left", "u")->showtable();
$content .= $table->buildtable(array("r3c1", "r3c2", "r3c3", "r3c4"), "center", "li")->showtable();
$content .= $table->buildtable(array("r4c1", "r4c2", "r4c3", "r4c4"), "left", "strike")->endtable();

echo $content;
?> 

 

 

 

So what do you think? Please do not hesitate to point out any problems you see from the codes, Id like to improve it as much as I can before using it in the applications of mine. Comments please?

 

[scootstah]

I guess I don't see any benefit to this over using plain HTML.

 

Also, no support for any attributes?

[Hall of Famer]

Well I will add attributes supports next. The reason to use this other than plain HTML is that, well, I have too many hard coded html tables in the script and the script files just look ugly.

[scootstah]

And this is not hard coded?

[Hall of Famer]

Well this is just a rough example, you can use it in different ways. For instance, retrieving database information and pass it to the table class.

 

And if you have any ideas on how to make a strictly non-hard coded table class, please lemme know and Id love to hear.

[KevinM1]

It seems a bit overwrought for something that could simply be created in a loop in a view.  Overhead without much utility.

 

That said, I have some other issues with it, too.  Why are there both a createTable and buildTable method?  From a usability POV, those methods tell anyone using the class the same thing.  Build and create are synonyms.  All you're doing is making a muddled API.

 

Also, from a usability POV, I don't see why both table data and table formatting options can't be passed into the constructor.  You have three methods doing what one should handle.

 

Pass options in as either an array or object, and handle them all in one place.  Right now, your methods handle arbitrary things (createTable and buildTable both handle alignment and CSS, while the constructor handles the background).  There's no clear design intent with that.

 

Remember DRY - Don't Repeat Yourself.  Code repetition is a symptom of bad design.  Simplify.

[Hall of Famer]

It seems a bit overwrought for something that could simply be created in a loop in a view.  Overhead without much utility.

 

That said, I have some other issues with it, too.  Why are there both a createTable and buildTable method?  From a usability POV, those methods tell anyone using the class the same thing.  Build and create are synonyms.  All you're doing is making a muddled API.

 

Also, from a usability POV, I don't see why both table data and table formatting options can't be passed into the constructor.  You have three methods doing what one should handle.

 

Pass options in as either an array or object, and handle them all in one place.  Right now, your methods handle arbitrary things (createTable and buildTable both handle alignment and CSS, while the constructor handles the background).  There's no clear design intent with that.

 

Remember DRY - Don't Repeat Yourself.  Code repetition is a symptom of bad design.  Simplify.

 

I see what you mean, thanks for the comment. And yeah, I could as well just have passed table formatting to the constructor. This way I will get rid of the createtable() method.

 

The reason why the buildtable() method is designed this way is that the alignments may change from each row, so are with the styles and maybe even background colors/images. The default option has all rows adopting the same alignment and style, but I try to make it flexible enough so that this can be changed at anywhere, anytime.

[Hall of Famer]

Changed createtable() to buildheader(), guess this way it wont create any confusion. I tried to bring everything from the original createtable() method to constructor, but the constructor ended up having way too many arguments and becomes ugly. Is there a way anyone can suggest such that I can reduce the number of arguments to no greater than five?

[Proletarian]

You could try passing an array as a parameter and make the array with associative keys.

[Hall of Famer]

You could try passing an array as a parameter and make the array with associative keys.

 

Thats a good suggestion, the problem is that some of these parameters are already arrays...

[thehippy]

You could look at PEAR/HTML_Table for some inspiration.

 

Personally the hard-coded styles I dislike very much, perhaps some default CSS classes would be alright, even better if those could be overridden.  Leave it up to the designer how it should look.

[KevinM1]

You could try passing an array as a parameter and make the array with associative keys.

 

Thats a good suggestion, the problem is that some of these parameters are already arrays...

 

Then pass in an object.  That's generally how the various HTML helper classes I've seen do things.

[Hall of Famer]

Thanks for the advice, much appreciated. I guess my next move is to collect all html table attributes such as align, border, cellpadding, cellspacing and width into an array or stdclass object called $attr and pass it to the constructor.  Its a better idea than what I had before, isnt it?

[scootstah]

Instead of explicitly stating which attributes can be placed, just allow everything. It's easier and you won't run the chance of missing something.

[Hall of Famer]

Well yeah, I tried to arrange all attributes such as width, border, bgcolor, cellpadding into one stdclass, which is created by calling the function getattributes() from the core functions file. It returns default properties for attributes of tables, forms, iframes and other html documents, these default properties can be set through admin control panel of my software(or simply hard-code them, though I do not like hard-coded stuff). Also note the table class has a static method setattributes(), which gives another degree of freedom since the default attributes can be changed in each script file. The new class file looks like this below:

 

<?php

class Table {
  public $tablename;
  private $columns = array();
  private $columnwidth;
  private $attributes;
  private $cells = array();  
  private $pointer = 0;
  private $header = "";
  private $body = array();
  private $footer = "";
  
  public function __construct($tablename, $columns, $columnwidth = "", $attributes = ""){
      // Create the table
      if(!is_array($columns)) die("Cannot create a table with only one column...");
      if(is_array($columnwidth) and count($columns) != count($columnwidth)) die("Specification of column width is incorrect, please report this to administrator.");
      $this->columns = $columns;
      if(is_array($columnwidth)){
         foreach($columnwidth as $width){
            $this->columnwidth[] = "width='{$width}'";
         }
      }
      elseif(!empty($columnwidth) and !is_array($columnwidth)) $this->columnwidth = " width='{$columnwidth}'";
      else $this->columnwidth = "";    
                  
      // Initiate the header with table attributes information
      
      $this->attributes = (!empty($attributes))?$attributes:getattributes()->table;
      $this->header = "<br><table";      
      $this->header .= (!empty($this->attributes->width))?" width='{$this->attributes->width}'":"";
      $this->header .= (!empty($this->attributes->border))?" border='{$this->attributes->border}'":"";
      $this->header .= (is_numeric($this->attributes->cellpadding))?" cellpadding='{$this->attributes->cellpadding}'":"";
      $this->header .= (is_numeric($this->attributes->cellspacing))?" cellspacing='{$this->attributes->cellspacing}'":"";
      $this->header .= (!empty($this->attributes->frame))?" frame='{$this->attributes->frame}'":"";
      $this->header .= (!empty($this->attributes->rules))?" rules='{$this->attributes->rules}'":"";
      $this->header .= (!empty($this->attributes->summary))?" summary='{$this->attributes->summary}'":"";
      $this->header .= (!empty($this->attributes->background))?" {$this->attributes->background[0]}='{$this->attributes->background[1]}'>":">";                  
  }
  
  public static function setattributes($attributes, $values){
      // This method allows us to modify default table attributes anywhere inside the script, it can be called with or without instantiating a table object.
      if(!is_array($values)) die("Cannot set table attributes without any inputs...");
      foreach($values as $key => $val){
         $attributes->$key = $val;
      }
      return $attributes;
  }

  public function getheader($align = "", $style = ""){
      // This method allows the page to show an unfinished table, incase control blocks, loops or forms need to be used
      $this->attributes->align = (!empty($align))?$align:$this->attributes->align;
      $this->attributes->style = (!empty($style))?$style:$this->attributes->style;
      $this->columns = $this->formattable($this->columns);
      
      $i = 0;
      $this->header .= "<tr>";
      foreach($this->columns as $column){
         $this->header .= (is_array($this->columnwidth))?"<th {$this->columnwidth[$i]}>{$column}</th>":"<th {$this->columnwidth}>{$column}</th>";
         $i++;
      }
      $this->header .= "</tr>";      
      return $this->header;
  }
  
  public function buildtable($cells, $align = "", $style = ""){
      // This method will build the body of our table, needs to be chained with showtable() or endtable()
      $index = count($this->body);
      $this->body[$index] .= "<tr>";       
      $this->attributes->align = (!empty($align))?$align:$this->attributes->align;
      $this->attributes->style = (!empty($style))?$style:$this->attributes->style;
      $this->cells = $this->formattable($cells);
       
      // Now it is time to construct a row of our table
      $i = 0;
      foreach($this->cells as $cell){
         $this->body[$index] .= "<td>{$cell}</td>";
         $i++;
      }
      $this->body[$index] .= "</tr>";
      return $this;      
  }
  
  private function formattable($text){
      // Thie method formats the content of tables with alignment or style, can only be called witin the table class
      if(!is_array($text)) die("Cannot format the table content.");
      if(!empty($this->attributes->style)){
         if(is_array($this->attributes->style) and count($this->attributes->style) != count($text)) die("Cannot specify the style of table columns...");
         for($i = 0; $i < count($text); $i++){
             $text[$i] = (is_array($this->attributes->style))?"<{$this->attributes->style[$i]}>{$text[$i]}</{$this->attributes->style[$i]}>":"<{$this->attributes->style}>{$text[$i]}</{$this->attributes->style}>";
         }
      }
      if(!empty($this->attributes->align)){
         if(is_array($this->attributes->align) and count($this->attributes->align) != count($text)) die("Cannot specify alignment of table columns...");
         for($i = 0; $i < count($text); $i++){
             $text[$i] = (is_array($this->attributes->align))?"<{$this->attributes->align[$i]}>{$text[$i]}</{$this->attributes->align[$i]}>":"<{$this->attributes->align}>{$text[$i]}</{$this->attributes->align}>";
         }
      }
      return $text;
  }

  public function resetpointer($index = 0){
      // This method allows the page to show an unfinished table, incase control blocks, loops or forms need to be used
      if(!is_int($index)) die("The index must be zero or a positive integer");
      elseif($this->pointer < $index) die("Failed to reset pointer...");
      else $this->pointer = $index;
  }

  public function showtable($showheader = "no"){
      // This method allows the page to show an unfinished table, incase control blocks, loops or forms need to be used
      $content = ($showheader == "yes")?$this->header:"";
      if(empty($this->body[$this->pointer])) die("The table has no more content to show...");
      for($i = $this->pointer; $i < count($this->body); $i++){
         $content .= $this->body[$this->pointer];
         $this->pointer++;
      }
      return $content;
  }
  
  public function endtable(){
      $content = "";
      // This method terminates the construction of a table
      if(!empty($this->footer)) die("The table has already ended...");
      for($i = $this->pointer; $i < count($this->body); $i++){
            $content .= $this->body[$this->pointer];
            $this->pointer++;
      }    
      $this->footer = "</table>";
      $content .= $this->footer;
      return $content;
  }
}
?> 

 

 

Any comments on this? I guess I've improved the script, but still have a long way to go until I call it a good design. Please lemme know what I should do next to make it better, thanks.

[scootstah]

You shouldn't be overwriting the classes vars when you call buildtable() or getheader(). As an end user, this would be aggravating. Every time I call one of those methods I'd have to reset any attributes I have explicitly set.

[Hall of Famer]

Well nope, you have completely misunderstood the concept. The default action is not to overwrite class vars, thats why the arguments such as $align and $style are initially defined as empty string. The script checks if the user wants to change the align and style for each row. If not, the class vars will not get changed and for most of the times it is the case. The properties only get changed if the user wants to have different attributes for each row. For instance, the first row can be centered while the second row has left-aligned text. These aint really default behaviors, just give users more degree of freedom. The fact that they can change some class properties at anytime they want does not mean they have to do it everytime a method like buildtable() is called.

 

$this->attributes->align = (!empty($align))?$align:$this->attributes->align;

 

Now if you take a look at this. The property attributes->align retains its original value if the user does not write any argument for the buildtable() method, but will be modified to whatever the user wants it to be if an argument is entered for $align.