Hi...
Originally posted by tshafer
Any critiques and optimizations of either the class or the access code is very welcome.
The first thing that strikes me is that very long constructor. Seems you are setting lot's and lot's of variables. Is there a way of grouping some of these into classes of their own. Small classes are a good thing. They can then each intialise themselves within their own constructors.
There is not yet a lot of abstraction (not unusual for a first cut).
More worrying is the choice of defaults. If a vital value is not set (such as the URL) then you should bomb out as quickly as possible and spectacularily as possible. Using trigger_error() should do the trick nicely. Having something silently work with a misleading default value will lead to bugs that are very hard to find. Set it to Null and do an error check for null on use.
If this thing is general purpose (classes should aspire to this), why is the template filename not taken in the constructor? It would make the client code much more descriptive and easier for an outsider to modify. It would be very be very obvious that the control code applied to a specific view.
Also worrying is the mention of databases in the template engine. What has a template class got to do with a database? You should definitely hack this. Any other solution is going to be better than that.
Apart from the constructor, the rest of the code is clear enough. Nice short descriptive methods. One of teh requirements of this group is that code be commented. I think this is bogus. Code should be easy to read and you use comments to clear up special cases and ambiguity. Most of your comments actually say nothing. I would strip the lot. Frees up time for refactoring the code 🙂.
yours, Marcus