3
\$\begingroup\$

I have an XML document that contains "ads" and includes information about the department name, image name, link to URL, and alt text. I have a PHP function that accepts two arguments, department name and the number of "ads" to display. It reads through the entire XML document, and stores the information in arrays if the department name matches what is passed to the argument or matches "all". What I have works, but it seems rather long and cumbersome. Is there a way to shorten it and improve performance?

PHP

function rightAds($deptName, $displayNumber) {
  //load xml
  /* --- Original loading, left in for completeness  
  $completeurl = "http://example.com/example.xml";
  $xml = simplexml_load_file($completeurl);    
  --- */

  //Loading without an HTTP call, as per Corbin's point
  $xml = simplexml_load_file(dirname(__FILE__)."/smallAdsByDepartment.xml");   

  $smallAds = $xml->smallAds;

  //number of ads that can be used for department
  $numberForDepartment = 0;     

  //loop through xml document, save the ads that are "all" or the department name
  foreach($smallAds as $ad) {

    //get department name
    $departmentName = $ad->departmentName;

    //if it is a valid department, add info to corresponding arrays        
    if((strpos($departmentName, $deptName) !== false) || (strpos($departmentName, "all") !== false)) {

      //PT exception.  If department name includes noPT, and provided name is pt, don't include the image
      if(($deptName == "pt" && strpos($departmentName, "noPT") !== false) || (strpos(dirname($_SERVER['PHP_SELF']), "pt") !== false && basename($_SERVER['REQUEST_URI']) == "admissions.php" &&  strpos($departmentName, "noPT") !== false )) {
        continue;                
      }
      //CLS exception. 
      if(($deptName == "cls" && strpos($departmentName, "noCLS") !== false)) {
        continue;
      }

      $name[$numberForDepartment] = $ad->name;
      $alt[$numberForDepartment] = $ad->alt;
      $linkTo[$numberForDepartment] = $ad->linkTo;
      $numberForDepartment++;
    }
  }

  //if $displayNumber is 1, or an invalid negative number, display random ad.
  if($displayNumber <= 1) { 
    $randKeys = mt_rand(0, $numberForDepartment);
    echo "<div class='adRight'><a href='$linkTo[$randKeys]'><img src='../../includes/images/adImages/$name[$randKeys]' alt='$alt[$randKeys]'></a></div>";
  }
  else {
        //get a list of randomly selected keys from the array
        $randKeys = range(0, (count($name) - 1));
        shuffle($randKeys);

        $numberOfAds = 0;

        //loop through and display image/link for each 
        foreach($randKeys as $adNumber) {     
            echo "<div class='adRight'><a href='" . $linkTo[$adNumber] . "' tabindex='0'><img src='../../includes/images/adImages/" . $name[$adNumber] . "' alt='" . $alt[$adNumber] . "'></a></div>";
            $numberOfAds++;
            if($numberOfAds == $displayNumber) { break; }
        }
   }
}

Example XML

<smallAds>
    <departmentName>all</departmentName>
    <name>smallAd3.jpg</name>
    <alt>Faculty Research</alt>
    <linkTo>http://example.com/research/index.php</linkTo>
</smallAds>
<smallAds>
    <departmentName>all, noPT</departmentName>
    <name>smallAd6.jpg</name>
    <alt>Advising</alt>
    <linkTo>http://example.com/advising.php</linkTo>
</smallAds>
<smallAds>
    <departmentName>all, noCLS</departmentName>
    <name>checkYouTube.png</name>
    <alt>Check us out on YouTube</alt>
    <linkTo>http://www.youtube.com/user/example</linkTo>
</smallAds>
\$\endgroup\$
  • \$\begingroup\$ Is using XML a requirement? \$\endgroup\$ – Corbin Apr 4 '14 at 17:39
  • \$\begingroup\$ I wouldn't be opposed to json, and there aren't that many ads so it wouldn't be too much of a hassle to convert it. \$\endgroup\$ – Matthew Johnson Apr 4 '14 at 17:45
3
\$\begingroup\$

If you can use any format, use serialized PHP, it's by far the fastest.

<?php

class Ad {

  public $alt;

  public $departmentName;

  public $linkTo;

  public $name;

}

function formatAd(Ad $ad) {
  return "<div class='adRight'><a href='{$ad->linkTo}'><img src='../../includes/images/adImages/{$ad->name}' alt='{$ad->alt}'></a></div>";
}

function rightAds($departmentName, $displayNumber) {
  static $ads = null;

  if (!$ads) {
    $ads = unserialize(file_get_contents(__DIR__ . "/smallAdsByDepartment.ser"));
  }

  $pt = $departmentName === "pt";

  /* @var $ad Ad */
  foreach ($ads as $delta => $ad) {
    if (strpos($ad->departmentName, $departmentName) !== false || strpos($departmentName, "all") !== false) {
      $noPT = strpos($ad->departmentName, "noPT") !== false;
      if (($pt && $noPT) || (strpos(__DIR__, "pt") !== false && basename($_SERVER["REQUEST_URI"]) === "admissions.php" && $noPT) || ($departmentName === "cls" && strpos($ad->departmentName, "noCLS") !== false)) {
        unset($ads[$delta]);
      }
    }
  }

  if ($displayNumber <= 1) {
    echo formatAd($ads[mt_rand(0, count($ads) - 1)]);
  }
  else {
    $keys = range(0, count($ads) - 1);
    shuffle($keys);
    $c = count($keys);
    for ($i = 0; $i < $c; ++$i) {
      echo formatAd($ads[$keys[$i]]);
      if ($i === $displayNumber) {
        break;
      }
    }
  }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.