[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