NYCPHP Meetup

NYPHP.org

[nycphp-talk] PHP patterns and antipatterns (was "rtrim broken?")

Paul Houle paul at devonianfarm.com
Wed Nov 1 11:55:44 EST 2006


David Krings wrote:
> Hi,
>
> I have a string that looks like this
> $text = 'blah, blah, blah, ';
> I want to use rtrim to cut off the last comma and the whitespace. So I use
> rtrim($text, ', ');
>   
    Creating delimited lists in strings is a common task,  and there are 
a number of patterns for doing it.  It's a task that PHP programmers in 
particular seem to struggle with.

    I don't like the pattern "create an invalid list,  then fix it up 
afterwards" and the related pattern "create an invalid list and expect 
the user of the list to deal with it".  All too often the "fix it up 
afterwards" gets forgotten or broken,  or the "user of the list" doesn't 
deal with the invalid list correctly.

    Until recently,  I used the following pattern

$list="";
foreach($values as $v) {
   $item=process($v);
   if ($list) {
      $list .= ",";
   }
   $list .=$item;
}

    Some will complain that I'm writing too many LoC.  More seriously,  
it's easy to code a pattern like this wrong.  One day I did this,  and 
decided to create a function to (i) reduce errors,  and (ii) reduce LoC.

function append_to_list(&$list,$item,$delimiter=",") {
    if($list) {
       $list .= ",";
    }
    $list .= $item;
}

    Now I write

$list="";
foreach($value as $v) {
   $item=process($v);
   append_to_list($list,$item);
}

    In terms of LoC,  this is as good as any of the other solutions,  
but has the advantage that the list is always in a valid state.  (For 
instance,  you can do a "return" to drop out of the loop at any time,  
the list is valid if an exception gets thrown,...)

    I added "append_to_list" to a file that contains utility functions 
that address my other PHP Pet Peeves...  You know...

$whatever="";
if (isset($_POST["whatever"])) {
   $whatever=$_POST["whatever"];
}

you might find this pattern coded up 100 different ways,  from the cute

$whatever=isset($_POST["whatever"]) ? $_POST["whatever"] : "";

to monstrosities that end up indenting hundreds of lines of code ten 
tabs because the programmer stacked a bunch of if(isset())'s.

Now I write

$whatever=array_get($_POST,"whatever");

this returns the empty string if $_POST["whatever"] is not set...  An 
optional third parameter lets you pick a different default value if you 
don't like "".

Another thing that drives me up the wall is

<input name=field1 value="<?php echo 
$user_supplied_string_that_may_contain_quotes ?>">

You ought to write

<?php echo htmlspecialchars($whatever) ?>

but that's painful.  It's nice to have

function q($string) {
   print htmlspecialchars($whatever);
}

So designers can write beautiful,  simple and correct code:

User name: <?php q($user_name) ?>

Alas,  there are two problems with this approach:

(1) It's tough to get other programmers on the team to be aware of this 
kind of library.  They end up scratching their heads when they see 
functions like this.  Getting them to take the initiative to actual use 
them is even harder.
(2) Defining functions in the global namespace pollutes the global 
namespace.  If you're pulling in libraries from multiple sources,  
there's a lot of risk that other people are going to claim names like 
"q()" and "array_get()".  It's not fun anymore if you have to write

PAULS_PET_PEEVES::q($user_name);

In some cases,  however,  you can do something like

$p=new PAULS_PET_PEEVES();
$p->q(...);

which at least leaves the option of changing $p to something else in 
places where you really need to.  This is particularly good for 
greenfield projects:  for instance,  I've got a struts-like 
"action-view-controller" framework where it's easy to pass a few utility 
objects with single-character names into the views that define 
anti-antipattern functions...  Views never end up with code like

<select name="myselect">
   <option value="1" <?php if($value==1) print " selected" ?>>Value 
1</option>
   ...
</select>

because you can just write

<select>
   <? $h->draw_select("myselect",$mychoices,$value) ?>
</select>






More information about the talk mailing list