Can this PHP code be improved?

Question

Is there a better way to do this simple task below? Like with an array or even another method?

<?PHP
// current way
if ($city != NULL) {
    $city = FilterALLHTML($city);
}
if ($state != NULL) {
    $state = FilterALLHTML($state);
}
if ($title != NULL) {
    $title = FilterALLHTML($title);
}
if ($division != NULL) {
    $division = FilterALLHTML($division);
}
?>

Here is my current function

function FilterALLHTML($document) {
    //old array line //"'<[\/\!]*?[^<>]*//?//>'si",// strip html
    $text = strip_tags($document);
    $search = array ("/f.?u.?c.?k/i",
        "/(s|$).?h.?i.?t/i",
        '/(potspace|mycrib|palbolt)/i');
    $text = preg_replace ($search, '', $text); 
    return $text;
}

---

UPDATE - Ok my new function after the suggestions from this post thanks guys

function FilterALLHTML($var) {
    //old array line //"'<[\/\!]*?[^<>]*//?//>'si",// strip html
    if ($var != null){
     $text = strip_tags($var);
     $search = array ("/f.?u.?c.?k/i",
         "/(s|$).?h.?i.?t/i",
         '/(potspace|mycrib|palbolt|pot space)/i');
     $text = preg_replace ($search, '', $text); 
     return $text;
    }
    return null;
}

Answer 1

Change your FilterALLHTML function to do the null check and have it return null? Then you can throw away all the ifs.

Example:

function FilterALLHTML($input)
{
    if ($input === null)
     return null;

    // Original code, I'll just use strip_tags() for a functional example
    return strip_tags($input);
}

Edit:

I felt like sharing an alternative to variable variables, as I don't really like the idea of using string literals instead of variable names. References all the way :)

function FilterALLHTML(&$text)
{
    if ($text !== null)
    {
     // Omitted regex bit for simplicity
     $text = strip_tags($text);
    }
}

$city = "<b>New York</b>";
$state = null;
$title = "<i>Mr.</i>";

$fields = array(&$city, &$state, &$title);
foreach ($fields as &$var)
    FilterALLHTML($var);

(note: FilterALLHTML implementation differs from first example)

Answer 2

I don't think you can improve the performance, but you can shorten the syntax, but it will end up being the same to the interpreter

<?PHP
    $city = ($city == NULL) ? "default value" : FilterALLHTML($city);
    $state = ($state == NULL) ? "default value" : FilterALLHTML($state);
    $title = ($title == NULL) ? "default value" : FilterALLHTML($title);
    $division = ($division == NULL) ? "default value" : FilterALLHTML($division);
?>

"default value" should be replaced with what you would like the value to be if the variable is null

Answer 3

Well, you could already consider writing a function because you do exactly the same thing four times.

Assuming FilterALLHTML is not a custom function.

function Filter($var)
{
    if ($var != null)
    {
        return FilterALLHTML($var);
    }
    return null;
}

Or just include the null check in the FilterALLHTML function and return null from there, if needed.

So, if you can change FilterALLHTML then you'd do it like this:

function FilterALLHTML($var)
{
    if ($var == null)
    {
        return null;
    }
    else
    {
        //do your filtering
       return $filteredVar;
    }
}

Answer 4

Yes, use PHP's variable variables.

$vars = array('city','state','title','division');
foreach($vars as $v) {
    if ($$v != null) $$v = FilterAllHTML($$v);
}

If you know for a fact that all the variables have been previously defined, then you don't need the null check. Otherwise, the null check will prevent E_NOTICE errors from triggering.

Answer 5

Adding to Thorarin's answer, you can change your filterall function in order to accept an array as input, and passing it by reference it will modify the arrays' content.

$tofilter = array($city,$state,$division,$title);


filterall($tofilter);

Answer 6

foreach (array('city', 'state', 'title', 'division') as $var) {
    if ($$var != null) {
        $$var = FilterALLHTML($$var);
    }
}

Like Thorarin I'd suggest having your FilterALLHTML function check for null instead though.

Answer 7

zombat's answer is the best, but I'd add that you shouldn't really be checking for null either. If for some reason FilterAllHTML has a problem with null values, which it shouldn't, put the check for null in the FilterAllHTML function definition.

$vars = array('city', 'state', 'title', 'division');
foreach($vars as $var) {
    $$var = FilterAllHTML($$var);
}

Answer 8

I didn't see it mentioned, you could always pass the parameters by reference to skip the repeated assignments:

function FilterALLHTML(&$var)
{
    if ($var == null)
    {
        $var = null;
    }
    else
    {
        $var = strip_tags($var);
    }
}

I believe you can also store references in the array but i haven't tried it.

foreach (array(&$city, &$state, &$title, &$division) as $var)
{
  FilterALLHTML($var);
}