OOP PHP: Where to put a "variable assigning" function

mtorbin · June 29, 2009

Hey all,

I'm trying to optimize a class file that I've written and I'm hoping you can help me. Here is the class file that I've written:

<?php
class TranslateXML {	
public function __construct($type,$url) {
	global $newXML;
	$this->scheduleXML	= "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<events>\n";
	$data			= simplexml_load_file($url);
	$counter		= 0;
	$sportSchedules = Array();
	switch($type) {
		case 'mlb':
			include 'class/class.MlbSchedule.php';
			foreach($data->{'sports-schedule'}->{'baseball-mlb-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new MlbSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->myGameID				= $sportSchedules[$counter]->getGameID();
				$this->myHomeTeamName 		= $sportSchedules[$counter]->getTeamName('home');
				$this->myHomeTeamCity		= $sportSchedules[$counter]->getTeamCity('home');
				$this->myVisitTeamName		= $sportSchedules[$counter]->getTeamName('visit');
				$this->myVisitTeamCity		= $sportSchedules[$counter]->getTeamCity('visit');
				$this->myGameDate			= $sportSchedules[$counter]->getGameDate();
				$this->myGameTime			= $sportSchedules[$counter]->getGameTime();
				$this->myVenueID			= $sportSchedules[$counter]->getVenueID();
				$this->myPerfID				= $sportSchedules[$counter]->getPerfID();
				$this->myGenre				= $sportSchedules[$counter]->getPcomGenre();
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			$this->scheduleXML .= "</events>\n";
			break;
		case 'nba':
			include 'class/class.NbaSchedule.php';
			foreach($data->{'sports-schedule'}->{'nba-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new NbaSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->myGameID				= $sportSchedules[$counter]->getGameID();
				$this->myHomeTeamName 		= $sportSchedules[$counter]->getTeamName('home');
				$this->myHomeTeamCity		= $sportSchedules[$counter]->getTeamCity('home');
				$this->myVisitTeamName		= $sportSchedules[$counter]->getTeamName('visit');
				$this->myVisitTeamCity		= $sportSchedules[$counter]->getTeamCity('visit');
				$this->myGameDate			= $sportSchedules[$counter]->getGameDate();
				$this->myGameTime			= $sportSchedules[$counter]->getGameTime();
				$this->myVenueID			= $sportSchedules[$counter]->getVenueID();
				$this->myPerfID				= $sportSchedules[$counter]->getPerfID();
				$this->myGenre				= $sportSchedules[$counter]->getPcomGenre();
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			$this->scheduleXML .= "</events>\n";
			break;
		case 'nfl':
			include 'class/class.NflSchedule.php';
			foreach($data->{'sports-schedule'}->{'football-nfl-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new NflSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->myGameID				= $sportSchedules[$counter]->getGameID();
				$this->myHomeTeamName 		= $sportSchedules[$counter]->getTeamName('home');
				$this->myHomeTeamCity		= $sportSchedules[$counter]->getTeamCity('home');
				$this->myVisitTeamName		= $sportSchedules[$counter]->getTeamName('visit');
				$this->myVisitTeamCity		= $sportSchedules[$counter]->getTeamCity('visit');
				$this->myGameDate			= $sportSchedules[$counter]->getGameDate();
				$this->myGameTime			= $sportSchedules[$counter]->getGameTime();
				$this->myVenueID			= $sportSchedules[$counter]->getVenueID();
				$this->myPerfID				= $sportSchedules[$counter]->getPerfID();
				$this->myGenre				= $sportSchedules[$counter]->getPcomGenre();
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			$this->scheduleXML .= "</events>\n";
			break;
		case 'nhl':
			include 'class/class.NhlSchedule.php';
			foreach($data->{'sports-schedule'}->{'hockey-nhl-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new NhlSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->myGameID				= $sportSchedules[$counter]->getGameID();
				$this->myHomeTeamName 		= $sportSchedules[$counter]->getTeamName('home');
				$this->myHomeTeamCity		= $sportSchedules[$counter]->getTeamCity('home');
				$this->myVisitTeamName		= $sportSchedules[$counter]->getTeamName('visit');
				$this->myVisitTeamCity		= $sportSchedules[$counter]->getTeamCity('visit');
				$this->myGameDate			= $sportSchedules[$counter]->getGameDate();
				$this->myGameTime			= $sportSchedules[$counter]->getGameTime();
				$this->myVenueID			= $sportSchedules[$counter]->getVenueID();
				$this->myPerfID				= $sportSchedules[$counter]->getPerfID();
				$this->myGenre				= $sportSchedules[$counter]->getPcomGenre();
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			break;
	}
}

function getNewXml() {return $this->scheduleXML;}

function buildNode() {
	$tempData = "\t\t<event_id>" . $this->myGameID . "</event_id>\n";
	$tempData .= "\t\t<event_title>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home)</event_title>\n";
	$tempData .= "\t\t<perf_id>" . $this->myPerfID . "</perf_id>\n";
	$tempData .= "\t\t<addl_perfs></addl_perfs>\n";
	$tempData .= "\t\t<venue_id>" . $this->myVenueID . "</venue_id>\n";
	$tempData .= "\t\t<event_desc>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home) [" . $this->myGameDate . ' @' . $this->myGameTime .  "]</event_desc>\n";
	$tempData .= "\t\t<genres>\n";
	$tempData .= "\t\t\t<genre>" . $this->myGenre . "</genre>\n";
	$tempData .= "\t\t</genres>\n";
	$tempData .= "\t\t<showtimes></showtimes>\n";
	$tempData .= "\t\t<schedule>\n";
	$tempData .= "\t\t\t<performance date=\"" . $this->myGameDate . "\">" . $this->myGameTime . "</performance>\n";
	$tempData .= "\t\t</schedule>\n";
	$tempData .= "\t</event>\n";
	return $tempData;
}
}
?>

As you can see in the switch statement, each of the four options does a lot of the same thing. I'd like to wrap those assignment statements in a function to cut down on size. However, whenever I try to add the function to the constructor, I get one of two errors (depending upon whether I use the prefix "$this->" with the call to the function:

Fatal error: Call to undefined method TranslateXML::setEventVars()

OR

Fatal error: Call to undefined function setEventVars()

I believe that the function needs to be encapsulated within the constructor but I think I'm missing something very small but VERY important. Any help would be much appreciated.

irvieto · June 29, 2009

Hi..

Would you post the setEventVars() method and in what line are you appending it..

Mark Baker · June 29, 2009

I believe that the function needs to be encapsulated within the constructor but I think I'm missing something very small but VERY important. Any help would be much appreciated.

No, it needs to be encapsulated within the class, not within the constructor

class TranslateXML {
private function setEventVars($var) {
	return strrev($var);
}

public function __construct($type,$url) {
	echo $this->setEventVars($type);
	echo $this->setEventVars($url);
}
}

OR

class TranslateXML {
private static function setEventVars($var) {
	return strrev($var);
}

public function __construct($type,$url) {
	echo $this::setEventVars($type);
	echo $this::setEventVars($url);
}
}

mtorbin · June 29, 2009

Would you post the setEventVars() method and in what line are you appending it..

Sure, sorry about that:

function setEventVars() {
$this->myGameID				= $sportSchedules[$counter]->getGameID();
$this->myHomeTeamName 		= $sportSchedules[$counter]->getTeamName('home');
$this->myHomeTeamCity		= $sportSchedules[$counter]->getTeamCity('home');
$this->myVisitTeamName		= $sportSchedules[$counter]->getTeamName('visit');
$this->myVisitTeamCity		= $sportSchedules[$counter]->getTeamCity('visit');
$this->myGameDate			= $sportSchedules[$counter]->getGameDate();
$this->myGameTime			= $sportSchedules[$counter]->getGameTime();
$this->myVenueID			= $sportSchedules[$counter]->getVenueID();
$this->myPerfID				= $sportSchedules[$counter]->getPerfID();
$this->myGenre				= $sportSchedules[$counter]->getPcomGenre();
}

I had put this AFTER the constructor but it seems like I need to put it BEFORE the constructor as Mark pointed out, yes?

- MT

mtorbin · June 29, 2009

UPDATE:

Mark, I posted the function above the constructor exactly as I have it listed above. While it didn't error out, it also didn't assign the vars. I have a feeling this is because it doesn't know what "this" is anymore, right? How would I correct this?

- MT

Mark Baker · June 29, 2009

I had put this AFTER the constructor but it seems like I need to put it BEFORE the constructor as Mark pointed out, yes?

Position in the class isn't relevant, before or after the constructor makes no difference.

Rules of scope still apply within classes: and both $sportSchedules and $counter are inaccessible to the method.

You need to pass $sportSchedules[$counter] as a parameter to the method

KevinM1 · June 29, 2009

When accessing static functions that are part of the class itself, you should use the 'self' keyword. As in:

self::functionName($arg1, $arg2)

mtorbin · June 29, 2009

Position in the class isn't relevant, before or after the constructor makes no difference.

Rules of scope still apply within classes: and both $sportSchedules and $counter are inaccessible to the method.

You need to pass $sportSchedules[$counter] as a parameter to the method

Ah, got it! It took some tweaking but I finally got the concept down. Here's the final script, enjoyably much leaner:

<?php
class TranslateXML {	
public function __construct($type,$url) {
	global $newXML;
	$this->scheduleXML	= "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<events>\n";
	$data			= simplexml_load_file($url);
	$counter		= 0;
	$sportSchedules = Array();
	switch($type) {
		case 'mlb':
			include 'class/class.MlbSchedule.php';
			foreach($data->{'sports-schedule'}->{'baseball-mlb-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new MlbSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->setEventVars($sportSchedules,$counter);
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			$this->scheduleXML .= "</events>\n";
			break;
		case 'nba':
			include 'class/class.NbaSchedule.php';
			foreach($data->{'sports-schedule'}->{'nba-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new NbaSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->setEventVars($sportSchedules,$counter);
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			$this->scheduleXML .= "</events>\n";
			break;
		case 'nfl':
			include 'class/class.NflSchedule.php';
			foreach($data->{'sports-schedule'}->{'football-nfl-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new NflSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->setEventVars($sportSchedules,$counter);
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			$this->scheduleXML .= "</events>\n";
			break;
		case 'nhl':
			include 'class/class.NhlSchedule.php';
			foreach($data->{'sports-schedule'}->{'hockey-nhl-schedule'}->{'game-schedule'} as $schedule) {
				$this->scheduleXML 			.= "\t<event>\n";
				$scheduleOBJ 				= new NhlSchedule($schedule,$data);
				$sportSchedules[$counter]	= $scheduleOBJ;
				$this->setEventVars($sportSchedules,$counter);
				$this->scheduleXML 			.= $this->buildNode();
				$counter++;
			}
			break;
	}
}

function getNewXml() {return $this->scheduleXML;}

function setEventVars($myOBJ,$myCount) {
	$this->myGameID			= $myOBJ[$myCount]->getGameID();
	$this->myHomeTeamName 	= $myOBJ[$myCount]->getTeamName('home');
	$this->myHomeTeamCity	= $myOBJ[$myCount]->getTeamCity('home');
	$this->myVisitTeamName	= $myOBJ[$myCount]->getTeamName('visit');
	$this->myVisitTeamCity	= $myOBJ[$myCount]->getTeamCity('visit');
	$this->myGameDate		= $myOBJ[$myCount]->getGameDate();
	$this->myGameTime		= $myOBJ[$myCount]->getGameTime();
	$this->myVenueID		= $myOBJ[$myCount]->getVenueID();
	$this->myPerfID			= $myOBJ[$myCount]->getPerfID();
	$this->myGenre			= $myOBJ[$myCount]->getPcomGenre();
}

function buildNode() {
	$tempData = "\t\t<event_id>" . $this->myGameID . "</event_id>\n";
	$tempData .= "\t\t<event_title>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home)</event_title>\n";
	$tempData .= "\t\t<perf_id>" . $this->myPerfID . "</perf_id>\n";
	$tempData .= "\t\t<addl_perfs></addl_perfs>\n";
	$tempData .= "\t\t<venue_id>" . $this->myVenueID . "</venue_id>\n";
	$tempData .= "\t\t<event_desc>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home) [" . $this->myGameDate . ' @' . $this->myGameTime .  "]</event_desc>\n";
	$tempData .= "\t\t<genres>\n";
	$tempData .= "\t\t\t<genre>" . $this->myGenre . "</genre>\n";
	$tempData .= "\t\t</genres>\n";
	$tempData .= "\t\t<showtimes></showtimes>\n";
	$tempData .= "\t\t<schedule>\n";
	$tempData .= "\t\t\t<performance date=\"" . $this->myGameDate . "\">" . $this->myGameTime . "</performance>\n";
	$tempData .= "\t\t</schedule>\n";
	$tempData .= "\t</event>\n";
	return $tempData;
}
}
?>

Guys thanks for all of your help. Nightslyr, I had one question about your post:

When accessing static functions that are part of the class itself, you should use the 'self' keyword. As in:

self::functionName($arg1, $arg2)

Are you saying instead of using "$this->" I should use "self::"? What is the difference between the two?

- MT

KevinM1 · June 29, 2009

'This' is used when dealing with individual objects. Example:

class Person
{
   private $name;
   private $age;

   public function __construct($name, $age)
   {
      $this->name = $name;
      $this->age = $age;
   }

   public function getName()
   {
      return $this->name;
   }
}

$bob = new Person('Bob', 23);
$sally = new Person('Sally', 45);

echo "Names: " . $bob->getName . ", " . $sally->getName();

$this->name's actual value varies from object to object. Static members are class-wide members. They don't belong to individual objects, but to the entire class. Example:

class StaticExample
{
   private static $num = 0;

   public function increment()
   {
      self::$num++;
   }

   public function getNum()
   {
      return self::$num;
   }
}

$example = new StaticExample();

$example->increment();
$example->increment();

echo $example->getNum(); // 2

More info: http://us.php.net/manual/en/language.oop5.static.php

aggrav8d · June 29, 2009

You might want to consider making setEventVars part of a base class and then the specifics for each type can be moved into a subclass. then you'd call (for example)

new NHLTranslateXML($url);

instead of

TranslateXML('nhl',$url);

It all depends on what you'll do with it in the future.

mtorbin · June 29, 2009

'This' is used when dealing with individual objects...

So, in reference to my script this line:

$this->setEventVars($sportSchedules,$counter);

should really be this line:

self::setEventVars($sportSchedules,$counter);

yes?

- MT

mtorbin · June 29, 2009

You might want to consider making setEventVars part of a base class and then the specifics for each type can be moved into a subclass. then you'd call (for example)

new NHLTranslateXML($url);

instead of

TranslateXML('nhl',$url);

It all depends on what you'll do with it in the future.

That's an interesting suggestion. I'm still very much learning OOP and the proper way to write things (thanks, BTW, to the great suggestions made at this forum). I always like to write things to make my life easier in the long run, though it was my understanding that if you can boil functions/classes down to their most simplistic format, it's better to have a bunch of things that each do one thing rather then a few things that each do many things. Is this the right way of looking at this?

- MT

KevinM1 · June 29, 2009

'This' is used when dealing with individual objects...

So, in reference to my script this line:
$this->setEventVars($sportSchedules,$counter);
should really be this line:
self::setEventVars($sportSchedules,$counter);
yes?

- MT

Only if setEventVars is a static function.

mtorbin · June 29, 2009

Only if setEventVars is a static function.

... which it is since it doesn't return a value; only processes data.

- MT

KevinM1 · June 29, 2009

You might want to consider making setEventVars part of a base class and then the specifics for each type can be moved into a subclass. then you'd call (for example)

new NHLTranslateXML($url);

instead of

TranslateXML('nhl',$url);

It all depends on what you'll do with it in the future.

That's an interesting suggestion. I'm still very much learning OOP and the proper way to write things (thanks, BTW, to the great suggestions made at this forum). I always like to write things to make my life easier in the long run, though it was my understanding that if you can boil functions/classes down to their most simplistic format, it's better to have a bunch of things that each do one thing rather then a few things that each do many things. Is this the right way of looking at this?

- MT

Yup, more or less. While an object can be complex in the way it performs its job, it shouldn't be overloaded with many jobs. It should do one or two things well, and that's it.

Regarding aggriv8d's suggestion, your constructor has what's sometimes called a 'bad code smell.' Your gigantic switch statement is a sign that you need to refactor your class. What you could do is subclass based on the sport you want. Something like:

class TranslateXML
{
   private $specificXML;

   public function __construct($type, $url)
   {
      switch ($type)
      {
         case 'mlb':
            $this->specificXML = new TranslateMLB($url);
            break;

         case 'nba':
            $this->specificXML = new TranslateNBA($url);
            break;

         case 'nfl':
            $this->specificXML = new TranslateNFL($url);
            break;

         case 'nhl':
            $this->specificXML = new TranslateNHL($url);
            break;
      }
   }
}

class TranslateMLB
{
   public __constructor($url)
   {
      /* build your XML */
   }
}

Only if setEventVars is a static function.

... which it is since it doesn't return a value; only processes data.

No...a static function has the keyword 'static' in front of it. A function without a return value is still just a function.

mtorbin · June 29, 2009

Regarding aggriv8d's suggestion, your constructor has what's sometimes called a 'bad code smell.' Your gigantic switch statement is a sign that you need to refactor your class. What you could do is subclass based on the sport you want.

Ok, let me see if I understand this correctly (and thanks for taking the time to help explain it; there's a beer waiting for you in Philly if you ever come to town). Currently I have the following classes:

- TranslateXML

- MlbSchedule

- NhlSchedule

- NbaSchedule

- NflSchedule

What you and Aggriv8d are suggesting is simply reformating the code in TranslateXML, not necessarily ingest the specific sport-related classes into the TranslateXML, correct (i.e. so that I'd end up with only one class file)?

- MT

mtorbin · June 29, 2009

Ok, here are the changes I made but the code is giving me errors:

<?php
class TranslateXML {	
public function __construct($type,$url) {
	$data			= simplexml_load_file($url);
	$counter		= 0;
	$sportSchedules = Array();
	$myXML;

	switch($type) {
		case 'mlb':
			include 'class/class.MlbSchedule.php';
			$this->myXML = new MlbXML($data);
			break;
		case 'nba':
			include 'class/class.NbaSchedule.php';
			$this->myXML = new NbaXML($data);
			break;
		case 'nfl':
			include 'class/class.NflSchedule.php';
			$this->myXML = new NflXML($data);
			break;
		case 'nhl':
			include 'class/class.NhlSchedule.php';
			$this->myXML = new NhlXML($data);
			break;
	}
}
}

class MlbXML {
	public function __construct($data) {
		$this->scheduleXML	= "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<events>\n";
		foreach($data->{'sports-schedule'}->{'baseball-mlb-schedule'}->{'game-schedule'} as $schedule) {
			$this->scheduleXML			.= "\t<event>\n";
			$scheduleOBJ 				= new MlbSchedule($schedule,$data);
			$sportSchedules[$counter]	= $scheduleOBJ;
			setEventVars($sportSchedules,$counter);
			$this->scheduleXML 			.= $this->buildNode();
			$counter++;
		}
		$this->scheduleXML .= "</events>\n";
	}
}

class NbaXML {
	public function __construct($data) {
		$this->scheduleXML	= "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<events>\n";
		foreach($data->{'sports-schedule'}->{'nba-schedule'}->{'game-schedule'} as $schedule) {
			$this->scheduleXML 			.= "\t<event>\n";
			$scheduleOBJ 				= new NbaSchedule($schedule,$data);
			$sportSchedules[$counter]	= $scheduleOBJ;
			self::setEventVars($sportSchedules,$counter);
			$this->scheduleXML 			.= $this->buildNode();
			$counter++;
		}
		$this->scheduleXML .= "</events>\n";
	}
}

class NflXML {
	public function __construct($data) {
		$this->scheduleXML	= "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<events>\n";
		foreach($data->{'sports-schedule'}->{'football-nfl-schedule'}->{'game-schedule'} as $schedule) {
			$this->scheduleXML 			.= "\t<event>\n";
			$scheduleOBJ 				= new NflSchedule($schedule,$data);
			$sportSchedules[$counter]	= $scheduleOBJ;
			self::setEventVars($sportSchedules,$counter);
			$this->scheduleXML 			.= $this->buildNode();
			$counter++;
		}
		$this->scheduleXML .= "</events>\n";
	}
}

class NhlXML {
	public function __construct($data) {
		$this->scheduleXML	= "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<events>\n";
		foreach($data->{'sports-schedule'}->{'hockey-nhl-schedule'}->{'game-schedule'} as $schedule) {
			$this->scheduleXML 			.= "\t<event>\n";
			$scheduleOBJ 				= new NhlSchedule($schedule,$data);
			$sportSchedules[$counter]	= $scheduleOBJ;
			self::setEventVars($sportSchedules,$counter);
			$this->scheduleXML 			.= $this->buildNode();
			$counter++;
		}
	}
}

function getNewXml() {return $this->scheduleXML;}

function setEventVars($myOBJ,$myCount) {
	$this->myGameID			= $myOBJ[$myCount]->getGameID();
	$this->myHomeTeamName 	= $myOBJ[$myCount]->getTeamName('home');
	$this->myHomeTeamCity	= $myOBJ[$myCount]->getTeamCity('home');
	$this->myVisitTeamName	= $myOBJ[$myCount]->getTeamName('visit');
	$this->myVisitTeamCity	= $myOBJ[$myCount]->getTeamCity('visit');
	$this->myGameDate		= $myOBJ[$myCount]->getGameDate();
	$this->myGameTime		= $myOBJ[$myCount]->getGameTime();
	$this->myVenueID		= $myOBJ[$myCount]->getVenueID();
	$this->myPerfID			= $myOBJ[$myCount]->getPerfID();
	$this->myGenre			= $myOBJ[$myCount]->getPcomGenre();
}

function buildNode() {
	$tempData = "\t\t<event_id>STATS" . $this->myGameID . "</event_id>\n";
	$tempData .= "\t\t<event_title>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home)</event_title>\n";
	$tempData .= "\t\t<perf_id>" . $this->myPerfID . "</perf_id>\n";
	$tempData .= "\t\t<addl_perfs></addl_perfs>\n";
	$tempData .= "\t\t<venue_id>" . $this->myVenueID . "</venue_id>\n";
	$tempData .= "\t\t<event_desc>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home) [" . $this->myGameDate . ' @' . $this->myGameTime .  "]</event_desc>\n";
	$tempData .= "\t\t<genres>\n";
	$tempData .= "\t\t\t<genre>" . $this->myGenre . "</genre>\n";
	$tempData .= "\t\t</genres>\n";
	$tempData .= "\t\t<showtimes></showtimes>\n";
	$tempData .= "\t\t<schedule>\n";
	$tempData .= "\t\t\t<performance date=\"" . $this->myGameDate . "\">" . $this->myGameTime . "</performance>\n";
	$tempData .= "\t\t</schedule>\n";
	$tempData .= "\t</event>\n";
	return $tempData;
}
?>

KevinM1 · June 29, 2009

What class contains setEventVars?

And, again, a function not returning a value != static function. A static function is a very specific thing...you'll know when you have one because the word 'static' will be visible before the word 'function' in its definition (see: http://us.php.net/manual/en/language.oop5.static.php). So, use $this->setEventVars(). You should also use $this->counter. It'd be greatly helpful if I could see all of your class code. It's a bit hard to follow what's going on without seeing all the properties and whatnot.

But, yes, you seem to be on the right track. The key is a concept called 'polymorphism'. Your client code will be dealing with the TranslateXML object's interface (its public functions). But, under the hood, those function calls will be delegated to the specific XML object that's stored in the $myXML data member. So, you'd have something like:

$xml = new TranslateXML('mlb', $url);
echo $xml->getResults();

class TranslateXML
{
   /* other data members and functions here */

   public function getResults()
   {
      return $this->myXML->getResults();
   }
}

Notice how the function call gets passed down to the more specialized object you're storing in $myXML. The client code doesn't care about the particulars of the operation, only that the right results are returned.

mtorbin · June 29, 2009

What class contains setEventVars?

I would think that, ideally, it would below to each of the sub-classes but writing that into each one wouldn't make a heck of a lot of sense because then I've just increased my file size unnecessarily, right? If I keep the function outside of the classes, then I need a way to pass in data to them.

It'd be greatly helpful if I could see all of your class code. It's a bit hard to follow what's going on without seeing all the properties and whatnot.

I can possibly send you everything but the actual data xml (since that is essentially proprietary data). However, unfortunately, this would have to happen "off-forum".

But, yes, you seem to be on the right track. The key is a concept called 'polymorphism'. Your client code will be dealing with the TranslateXML object's interface (its public functions). But, under the hood, those function calls will be delegated to the specific XML object that's stored in the $myXML data member.

Thanks. I think if I take a moment and re-write instead of patch I might see a much clearer picture.

- MT

KevinM1 · June 29, 2009

Hmm...

I think the best way to tackle it is this:

1. Create an abstract base class that will contain all of the shared functionality (both data members and functions) that the rest of the classes need. Abstract classes are special classes that cannot be instantiated themselves, and are used just for this kind of thing. They're easy to create:

abstract class BaseXML { }

But, again, doing something like:

$x = new BaseXML();

Won't work, and will generate an error.

2. Put all of that shared stuff I mentioned in step 1 in the class. This works just like creating a normal, non-abstract class. So, all the data members that are used in every other class should go there, and the setEventVars function should be there.

3. Create your child classes. You should have five of them - the generalized TranslateXML class (which we'll use in exactly the same way as before), and the specialized, sport-specific classes. What goes into these classes are all the things that aren't shared. So, a rough sketch should look like:

abstract class BaseXML
{
   /* data members that all other classes use - make them PROTECTED */

   public function setEventVars(/* argument list */) { /* function body */ }
}

class TranslateXML extends BaseXML
{
   private $myXML;

   public function __construct($type, $url)
   {
      switch($type) { /* same switch code as before */ }
   }
}

class XMLMlb extends BaseXML
{
   public function __construct() { }
}

You ensure that all child classes have access to what they need by stuffing the shared functionality as high on the inheritance chain as it can go. That way, all the children automatically have those abilities, without the need for you to specify them in each class. It's only the things that vary (example: TranslateXML's $myXML data member) that need to be defined in the children.

I hope this makes sense. OOP is a bit hard to get into, at least, if you want to do it right. If you can, do a google search on 'PHP OOP inheritance' - that should help explain what I'm trying to do.

mtorbin · June 29, 2009

[reading tutorials and responding to this blog at the same time]

So I can understand one level of abstraction but why two? In other words, why can't TranslateXML also contain the function setEventVars and all of the variables that are global?

- MT

aggrav8d · June 29, 2009

pseudocode

class TranslateXML {
  __construct($url='') {
  }
  function setEventVars() {}
}

class NHL_TranslateXML extends TranslateXML {
  __construct($url='') {
    // nhl specific stuff here
  }
}

class NFL_TranslateXML extends TranslateXML {
  __construct($url='') {
    // nfl specific stuff here
  }
}

// ... later ...
// class factory
function GameClassFactory($type,$url) {
  switch($type) {
  case 'nhl': return new NHL_TranslateXML($url);
  case 'nfl': return new NFL_TranslateXML($url);
  default: return null;
  }
}

KevinM1 · June 29, 2009

[reading tutorials and responding to this blog at the same time]

So I can understand one level of abstraction but why two? In other words, why can't TranslateXML also contain the function setEventVars and all of the variables that are global?

- MT

You could have TranslateXML be the parent class. Nothing wrong with that.

mtorbin · June 30, 2009

Just to update everyone on the thread, I'm going to rewrite the class and repost but it may take me a day or so. Thanks again for all of your help. This is one of those forums I really dig because I've learned so much here.

-MT

mtorbin · June 30, 2009

This is the "skeleton" I'm starting with. Please let me know if this looks about right:

<?php
class TranslateXML {	
public function __construct($type,$url) {

}

function getNewXml() {return $this->scheduleXML;}

function setEventVars($myOBJ,$myCount) {
	$this->myGameID			= $myOBJ[$myCount]->getGameID();
	$this->myHomeTeamName 	= $myOBJ[$myCount]->getTeamName('home');
	$this->myHomeTeamCity	= $myOBJ[$myCount]->getTeamCity('home');
	$this->myVisitTeamName	= $myOBJ[$myCount]->getTeamName('visit');
	$this->myVisitTeamCity	= $myOBJ[$myCount]->getTeamCity('visit');
	$this->myGameDate		= $myOBJ[$myCount]->getGameDate();
	$this->myGameTime		= $myOBJ[$myCount]->getGameTime();
	$this->myVenueID		= $myOBJ[$myCount]->getVenueID();
	$this->myPerfID			= $myOBJ[$myCount]->getPerfID();
	$this->myGenre			= $myOBJ[$myCount]->getPcomGenre();
}

function buildNode() {
	$tempData = "\t\t<event_id>STATS" . $this->myGameID . "</event_id>\n";
	$tempData .= "\t\t<event_title>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home)</event_title>\n";
	$tempData .= "\t\t<perf_id>" . $this->myPerfID . "</perf_id>\n";
	$tempData .= "\t\t<addl_perfs></addl_perfs>\n";
	$tempData .= "\t\t<venue_id>" . $this->myVenueID . "</venue_id>\n";
	$tempData .= "\t\t<event_desc>" . $this->myVisitTeamCity . ' ' . $this->myVisitTeamName . ' (away) vs. ' . $this->myHomeTeamCity . ' ' . $this->myHomeTeamName . " (home) [" . $this->myGameDate . ' @' . $this->myGameTime .  "]</event_desc>\n";
	$tempData .= "\t\t<genres>\n";
	$tempData .= "\t\t\t<genre>" . $this->myGenre . "</genre>\n";
	$tempData .= "\t\t</genres>\n";
	$tempData .= "\t\t<showtimes></showtimes>\n";
	$tempData .= "\t\t<schedule>\n";
	$tempData .= "\t\t\t<performance date=\"" . $this->myGameDate . "\">" . $this->myGameTime . "</performance>\n";
	$tempData .= "\t\t</schedule>\n";
	$tempData .= "\t</event>\n";
	return $tempData;
}
}

class MlbXML extends TranslateXML {

}

class NbaXML extends TranslateXML {

}

class NflXML extends TranslateXML {

}

class NhlXML extends TranslateXML {

}
?>

- MT

Sign In

OOP PHP: Where to put a "variable assigning" function

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information