Welcome back! I have been doing OOP for a very long time and I always love to pass on what I have learned (had beaten into my head). Please take these suggestions as that, they are not gospel, just habits I have picked up over time.
I have a few suggestions:
1) Break out rendering of separate elements to their own objects for extensibility
2) Don't pass arguments into the constructor, use getter/setter methods. This way, later if you want to add parameters, you dont have to retro all your code to use the new parameters.
3) Change your privates to "protected" so you can overload (sub-class) this class later (see decorator pattern). Also, I use an underscore in the method name to remind me that a function is protected/private vs. public.
4) Use a factory to instance the child classes (will explain later)
5) Use exceptions to bubble problems to the application
6) Don't tie your class to a database result, sometimes we have to query the database, do some computations, massaging, etc. Well, then you dont have a solution. Then you can sub-class this application to load the data from a query.
7) Make all your rendering functions separate functions so they can be overloaded in a sub-class
8) Look into PHPDOC for documenting your code :-)
PART 1:
You are creating a rendering class. Rendering classes almost always grow into "god classes" when implemented this way. You are going to find you need more formatting, for example, text alignment. Then checkboxes so that I could select records and do something useful. Then number formatting (round to 2 decimal places and add commas). Then font styles (bold, italic, colors, etc). Then subtotaling/summary lines, etc, etc, etc.
What should you do? break out the functions of rendering to sub-objects. By doing that, if you need a different type of field rendering, you can create a new Cell class with special formatting that does not effect the other cell class. I can provide some examples if you would like. Let me give you a small example:
// ... within the class
public function defineCell( $strCellName, $strType )
{
// this is the factory creation pattern (PART 4)
$this->_arrCellClasses[ $strCellName ] = cCellMaker::makeCell( $strType );
return $this->_arrCellClasses[ $strCellName ];
}
so what is going on here. Well, we are constructing a new Cell by the type. So we could have. a cMoneyCell, an cHrefCell, a cCheckBoxCell, or a cGenericCell. All separate objects.
Ok, so what does that buy us? Also why are you returning the newly created object? Well this is why.
// ... in the application
$objTable = new MakeSortTable();
// DEFINE a MoneyCell:
$objMoneyCell = $objTable->defineCell( 'GoodDollars', 'Money' );
// set the decimal precision to 4 decimal places
$objMoneyCell->setPrecision( 4 );
// Some currencies use a different thousands separator and symbol:
$objMoneyCell->setThousandsSeparator( ';' );
$objMoneyCell->setCurrencySymbol( '$' );
Now, the code will default, if you do not declare a specific class to the GenericCell so that you do not have to define each and every cell when you use this class, just the exceptions. Kinda like this:
// ... in the MakeSortTable class
public function renderTable()
{
// Make these separate methods in case later you
// want to overload them in a sub-class:
// This is actually PART 7
$this->_mRenderTable();
$this->_mRenderCaption();
$this->_mRenderHeaders();
$this->_mRenderData();
$this->_mRenderCloseTable();
}
protected function _mRenderData()
{
if ( ! count( $this->_arrData ) )
{
throw new Exception( 'There was an error, The data '.
'for this table has not been set. '.
'Please contact technical support.' );
}
$this->_mCreateGenericCells();
// Loop the rows
foreach( $this->_arrData as $arrDataRow )
{
foreach( $arrDataDow as $strCell=>$strValue )
{
$this->_arrCellClasses[ $strCell ]->setValue( $strValue );
echo $this->_arrCellClasses[ $strCell ]->render();
}
}
}
// Generates the Generic Cell objects if the application did not declare a class
protected function _mCreateGenericCells()
{
foreach( $this->_arrData[0] as $strCell=>$strValue )
{
if( ! isset( $this->_arrCellClasses[ $strCell ] )
{
$this->defineCell( $strCell, 'Generic' );
}
}
}
phew, that is a long way to go just to render a table, but what you should see is that you can extend this to do almost anything you want. This is a small view into my Reporting application. Not only can it render out the data a million different ways, it can render out in multiple formats ( HTML, XML, PDF, excel, Word, DocBook) because the output is "hidden" (read abstracted) from the main class into the "render" methods of the sub-objects.
One last point, this is a big pet pev for me. Try not to pass arguments into constructors. This is an area of controversy, but let me explain my rational. Lets say today, your constructor takes 2 args. Well, tomorrow you might want to change that to say 3, then 4 then 6, then 10. Well, then you have to remember what the order of the arguments are to use them, and you may not need them in all cases. So I propose using getter/setter functions:
public function setTableId( $strId )
{
$this->_strId = $strId;
}
public function getTableId()
{
return $this->_strId;
}
This also allows you to get the id or title back out of your object. The way you have it now, it gets embedded. You also might have an occasion where you want to render the same table 2 times just with a different id. You could then with the same object just call the setTableId with a new Id and render again, instead of re-initializing or re-instancing the same object. This is another form of abstraction. Good OOP practice.
I hope this helps, and does not confuse. I am just trying to give you some insight into how to make your base classes like this more extensible.
Good luck,
Mike
