HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Displaying random "ads" from an XML document

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomxmldisplayingdocumentfromads

Problem

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[$numberForDepa

Solution

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

linkTo}'>name}' alt='{$ad->alt}'>";
}

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;
      }
    }
  }
}

Code Snippets

<?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;
      }
    }
  }
}

Context

StackExchange Code Review Q#46302, answer score: 3

Revisions (0)

No revisions yet.