[ciapug] Peer review of PHP code

David Champion ciapug@cialug.org
Tue, 13 Apr 2004 11:14:04 -0500


Claus wrote:

> On 4/12/2004 4:28 PM, Barry Von Ahsen wrote:
>
>> I personally turn the interpreter on and off as necessary (ie. not 
>> writing html with php), but that's a personal preference (plus then 
>> the syntax checker checks the html syntax, not the php).  I know this 
>> was bad performance-wise with ASP2.0/IIS4, but I haven't heard of any 
>> problems with php.
>
>
> I've been arguing about this with myself a bit, too.  Somehow the 
> consistant use of php appealed to me better than going in and out of 
> PHP.  Especially since that page is more logic driven.
>
> The page that actually presents the newsletters to the viewer is HTML 
> with PHP embedded.  There the program/parse flow is straight forward.
>
> As far as HTML syntax checker goes, I would run them after the web 
> server has parsed/served the page since that's what is actually passed 
> to the browser.  But the reality is that I never bother with syntax 
> checkers.  My most advanced HTML tag is the <table> tag.  Never really 
> had time to embrace the CSS front although it looks promising.
>
>   Claus

The biggest arguement for this is the "seperating design from logic" 
point. For instance, if you're a coder working with a design team, 
they're probably going to want to open pages up in Dreamweaver or some 
other layout program and mess with the HTML parts of the page. If you 
imbed everything inside PHP echo commands, they can't do that.

I wouldn't be suprised if there were a minor performance hit by turning 
PHP on / off several times, but I doubt it's very much of one.

A couple of other comments:

You have some <option> lists with hard-coded date values. While you've 
given yourself some pretty good headroom on the years, what would happen 
if this code were still in production after the last year? You'd have to 
modify the code to add years. And will you need to show the past years? 
It's really pretty trivial to use PHP's date functions to generate these 
lists. I almost never use hard-coded <option> lists. IMHO, once you've 
written something like this, you should never have to / want to go back 
and maintain it for this kind of change.

Also... that switch function to return the month name could be 
eliminated using date() - see : 
http://us4.php.net/manual/en/function.date.php

-dc