combining PHP functions

Question

I've inherited some php that parses an xml file to populate a page full of unordered lists and am wondering if there is a way to consolidate the php functions to make them more efficient.

There are 25 or so functions like the following:

function oaAccounting(){

  // load SimpleXML
  $term = new SimpleXMLElement('training_list.xml', null, true);
  echo <<<EOF
  <ul>
EOF;

  foreach($term as $term)
  {
     if(preg_match("/accounting/i", $term->keyword)){
        echo <<<EOF
        <li>{$term->name}</li>
EOF;
     }
  }
  echo '</ul>';
}

each one scans the xml file for the term/keyword it's searching for and adds the term as a list element to an unordered list specific to that function. The next function does the same thing but for a different term/keyword and adds it to a separate unordered list.

is there a way to combine all this to prevent having to do the foreach and if 25 times in a row?

Thanks!

Rewrite. Did you realize, that $term is overwritten in every iteration? And why regular expressions? XML has xpath for that. — KingCrunch
– KingCrunch, Commented Apr 19, 2012 at 19:59

Madara's Ghost · Accepted Answer · 2012-04-19 20:00:35Z

1

Abstract your function.

Pass the file and the search terms as arguments to the function, and work with that.

Something like:

function parse_xml_file($file, $terms)

Then use pass $file and $terms when calling the function.

For more information, see Teh big bad manual.

answered Apr 19, 2012 at 20:00

Madara's Ghost

176k52 gold badges274 silver badges314 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

John Conde · Accepted Answer · 2012-04-19 19:57:36Z

0

This revised function takes one parameter, the word to look for, and then uses it in the preg_match() function. So to use it all you need to do is call it with the term you're searching for. This should replace all of the hard coded functions.

function searchXML($search){

    // load SimpleXML
    $terms = new SimpleXMLElement('training_list.xml', null, true);
    echo <<<EOF
    <ul>
    EOF;
    foreach($terms as $term)
    {
        if(preg_match("/$search/i", $term->keyword)){
            echo <<<EOF
            <li>{$term->name}</li>
        EOF;
    }}
    echo '</ul>';
}

answered Apr 19, 2012 at 19:57

John Conde

220k100 gold badges464 silver badges504 bronze badges

6 Comments

Madara's Ghost Over a year ago

Wrong, you would override the $term argument with the SimpleXMLElement.

John Conde Over a year ago

I just noticed my error and corrected it while formatting the code. I dunno if that was worth a vote down. You could have edited the answer and fixed it instead.

Madara's Ghost Over a year ago

I could, but I don't like to edit other people's code. As it changes the meaning of the answer. I would change typos or grammar mistakes when I spot them, or fix indention in case your code needs it (but I won't change the code itself!). DV removed, happy browsing! :D

Madara's Ghost Over a year ago

@KingCrunch: I meant the $term argument, him using the same variable in the foreach is a whole different issue.

KingCrunch Over a year ago

@Truth Me too ;) Read the question above. There the OP did exactly this. Did say, that it makes anything better, but at least it's not (directly) JohnCondes mistake.

|

Collectives™ on Stack Overflow

combining PHP functions

2 Answers 2

Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related