Skip to main content
Added example of lazy loading individual properties.; added 127 characters in body
Source Link
Tim Lytle
  • 251
  • 1
  • 4

You can avoid navigating the SimpleXML object multiple times for the same data using the same basic concept the other lazy loading functions use (getXML(), getResponse()).

Instead of using object properties, I just put a static variable in the method - since the location can not change, there's no never a need to unset all the data.

However, if the location could change, the same could be accomplished with $this->data[$property] - obviously resetting $this->data to array() every time the location is changed.

Here's an improved getTemp():

  public function getTemp($type = 'f')
  {
    static $temp = array();
    
    //validate type
    if(!in_array($type, array('f','c'))){
      throw Exception('invalid temp type: ' . $type);
    }
    
    if(isset($temp[$type])){
      return $temp[$type];
    }

    $element = 'temp_' . $type;
    //make sure there's data
    if(!isset($this->getCondition()->{$element}['data'])){
      throw new Exception('could not find temp');
    }

    //cast as float and return
    $temp[$type] = $this->getCondition()->{$element}['data'];
    return $temp[$type];
  }

I'd be interested in seeing if there's any significant performance gained over navigating the SimpleXML object each time.

You can avoid navigating the SimpleXML object multiple times for the same data using the same basic concept the other lazy loading functions use (getXML(), getResponse()).

Instead of using object properties, I just put a static variable in the method - since the location can not change, there's no never a need to unset all the data.

However, if the location could change, the same could be accomplished with $this->data[$property] - obviously resetting $this->data to array() every time the location is changed.

Here's an improved getTemp():

  public function getTemp($type = 'f')
  {
    static $temp = array();
    
    //validate type
    if(!in_array($type, array('f','c'))){
      throw Exception('invalid temp type: ' . $type);
    }
    
    if(isset($temp[$type])){
      return $temp[$type];
    }

    $element = 'temp_' . $type;
    //make sure there's data
    if(!isset($this->getCondition()->{$element}['data'])){
      throw new Exception('could not find temp');
    }

    //cast as float and return
    $temp[$type] = $this->getCondition()->{$element}['data'];
    return $temp[$type];
  }

I'd be interested in seeing if there's any significant performance gained over navigating the SimpleXML object each time.

added exception to fetchData()
Source Link
Tim Lytle
  • 251
  • 1
  • 4
<?php
class GoogleWeather{
  protected $api = 'http://www.google.com/ig/api?weather=';
  protected $xml;
  protected $response;
  protected $location;

  public function __construct( $location) {
    // Set location
    $this->location = $location;
  }

  public function setResponse($response)
  {
    $this->response = $response;
  }

  public function getResponse()
  {
    //if the xml hasn't been fetched, then fetch it
    if(empty($this->response)){
      $this->setResponse($this->fetchData());
    }
    return $this->response;
  }

  //was get_xml renamed to avoid confusion
  public function fetchData()
  {
    // Download raw XML to be parsed.
    $ch = curl_init($this->api . urlencode($this->location));

    // I don't know why I altered the useragent. It must have been for a good reason. Oh well.
    curl_setopt($ch, CURLOPT_USERAGENT, 'Mozilla/5.0 (Windows NT 6.1; rv:5.0.1) Gecko/20100101 Firefox/5.0.1');
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
    $rawXML = curl_exec($ch); 

    if (!$rawXML){
      return//check false;the curl error for a better message
      throw new Exception('could not fetch data'); 
    }
    curl_close($ch);
    return $rawXML;
  }

  public function getXml()
  {
    if(empty($this->xml)){
      try{
        $this->xml = new SimpleXMLElement($this->getResponse());
      } catch (Exception $e) {
        //there's no real recovery here, except maybe to retry fetching the data
        throw new Exception('bad response from from API');
      }

      //check for 'problem_cause' element
      if(isset($this->xml->weather->problem_cause)){
        throw new Exception('API responded with error');
      }
    }
    return $this->xml;
  }

  public function getCondition()
  {
    //make sure there's data
    if(!isset($this->getXml()->weather->current_conditions)){
      //you could throw an exception and assume any code using this is 
      //calling it in a try block
      throw new Exception('could not find conditions');
    }

    return $this->getXml()->weather->current_conditions;
  }

  public function getTemp($type = 'f')
  {
    //validate type
    if(!in_array($type, array('f','c'))){
      throw Exception('invalid temp type: ' . $type);
    }

    $element = 'temp_' . $type;
    //make sure there's data
    if(!isset($this->getCondition()->{$element}['data'])){
      throw new Exception('could not find temp');
    }

    //cast as float and return
    return (float) $this->getCondition()->{$element}['data'];
  }
}
<?php
class GoogleWeather{
  protected $api = 'http://www.google.com/ig/api?weather=';
  protected $xml;
  protected $response;
  protected $location;

  public function __construct( $location) {
    // Set location
    $this->location = $location;
  }

  public function setResponse($response)
  {
    $this->response = $response;
  }

  public function getResponse()
  {
    //if the xml hasn't been fetched, then fetch it
    if(empty($this->response)){
      $this->setResponse($this->fetchData());
    }
    return $this->response;
  }

  //was get_xml renamed to avoid confusion
  public function fetchData()
  {
    // Download raw XML to be parsed.
    $ch = curl_init($this->api . urlencode($this->location));

    // I don't know why I altered the useragent. It must have been for a good reason. Oh well.
    curl_setopt($ch, CURLOPT_USERAGENT, 'Mozilla/5.0 (Windows NT 6.1; rv:5.0.1) Gecko/20100101 Firefox/5.0.1');
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
    $rawXML = curl_exec($ch); 

    if (!$rawXML){
      return false;
    }
    curl_close($ch);
    return $rawXML;
  }

  public function getXml()
  {
    if(empty($this->xml)){
      try{
        $this->xml = new SimpleXMLElement($this->getResponse());
      } catch (Exception $e) {
        //there's no real recovery here, except maybe to retry fetching the data
        throw new Exception('bad response from from API');
      }

      //check for 'problem_cause' element
      if(isset($this->xml->weather->problem_cause)){
        throw new Exception('API responded with error');
      }
    }
    return $this->xml;
  }

  public function getCondition()
  {
    //make sure there's data
    if(!isset($this->getXml()->weather->current_conditions)){
      //you could throw an exception and assume any code using this is 
      //calling it in a try block
      throw new Exception('could not find conditions');
    }

    return $this->getXml()->weather->current_conditions;
  }

  public function getTemp($type = 'f')
  {
    //validate type
    if(!in_array($type, array('f','c'))){
      throw Exception('invalid temp type: ' . $type);
    }

    $element = 'temp_' . $type;
    //make sure there's data
    if(!isset($this->getCondition()->{$element}['data'])){
      throw new Exception('could not find temp');
    }

    //cast as float and return
    return (float) $this->getCondition()->{$element}['data'];
  }
}
<?php
class GoogleWeather{
  protected $api = 'http://www.google.com/ig/api?weather=';
  protected $xml;
  protected $response;
  protected $location;

  public function __construct( $location) {
    // Set location
    $this->location = $location;
  }

  public function setResponse($response)
  {
    $this->response = $response;
  }

  public function getResponse()
  {
    //if the xml hasn't been fetched, then fetch it
    if(empty($this->response)){
      $this->setResponse($this->fetchData());
    }
    return $this->response;
  }

  //was get_xml renamed to avoid confusion
  public function fetchData()
  {
    // Download raw XML to be parsed.
    $ch = curl_init($this->api . urlencode($this->location));

    // I don't know why I altered the useragent. It must have been for a good reason. Oh well.
    curl_setopt($ch, CURLOPT_USERAGENT, 'Mozilla/5.0 (Windows NT 6.1; rv:5.0.1) Gecko/20100101 Firefox/5.0.1');
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
    $rawXML = curl_exec($ch); 

    if (!$rawXML){
      //check the curl error for a better message
      throw new Exception('could not fetch data'); 
    }
    curl_close($ch);
    return $rawXML;
  }

  public function getXml()
  {
    if(empty($this->xml)){
      try{
        $this->xml = new SimpleXMLElement($this->getResponse());
      } catch (Exception $e) {
        //there's no real recovery here, except maybe to retry fetching the data
        throw new Exception('bad response from from API');
      }

      //check for 'problem_cause' element
      if(isset($this->xml->weather->problem_cause)){
        throw new Exception('API responded with error');
      }
    }
    return $this->xml;
  }

  public function getCondition()
  {
    //make sure there's data
    if(!isset($this->getXml()->weather->current_conditions)){
      //you could throw an exception and assume any code using this is 
      //calling it in a try block
      throw new Exception('could not find conditions');
    }

    return $this->getXml()->weather->current_conditions;
  }

  public function getTemp($type = 'f')
  {
    //validate type
    if(!in_array($type, array('f','c'))){
      throw Exception('invalid temp type: ' . $type);
    }

    $element = 'temp_' . $type;
    //make sure there's data
    if(!isset($this->getCondition()->{$element}['data'])){
      throw new Exception('could not find temp');
    }

    //cast as float and return
    return (float) $this->getCondition()->{$element}['data'];
  }
}
Source Link
Tim Lytle
  • 251
  • 1
  • 4

Use methods to get the data, this is a kind of dependency injection and will allow you to test the code with out having to hit google's server everytime. It will also avoid the if($this->_isParsed) on all your functions.

Additionally, move the 'parsing' - which is really just navigating the XML structure - to the functions that return the data. This removes your parse_xml function (which would now be just a function that creates the SimpleXML object), and allows you to throw exceptions only when requested data is missing/unexpected (or return just return false, if that's how you want your client to act).

Here's some example code.

I tend to use camelCase, I'm not trying to change your style, it's just second nature.

<?php
class GoogleWeather{
  protected $api = 'http://www.google.com/ig/api?weather=';
  protected $xml;
  protected $response;
  protected $location;

  public function __construct( $location) {
    // Set location
    $this->location = $location;
  }

  public function setResponse($response)
  {
    $this->response = $response;
  }

  public function getResponse()
  {
    //if the xml hasn't been fetched, then fetch it
    if(empty($this->response)){
      $this->setResponse($this->fetchData());
    }
    return $this->response;
  }

  //was get_xml renamed to avoid confusion
  public function fetchData()
  {
    // Download raw XML to be parsed.
    $ch = curl_init($this->api . urlencode($this->location));

    // I don't know why I altered the useragent. It must have been for a good reason. Oh well.
    curl_setopt($ch, CURLOPT_USERAGENT, 'Mozilla/5.0 (Windows NT 6.1; rv:5.0.1) Gecko/20100101 Firefox/5.0.1');
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
    $rawXML = curl_exec($ch); 

    if (!$rawXML){
      return false;
    }
    curl_close($ch);
    return $rawXML;
  }

  public function getXml()
  {
    if(empty($this->xml)){
      try{
        $this->xml = new SimpleXMLElement($this->getResponse());
      } catch (Exception $e) {
        //there's no real recovery here, except maybe to retry fetching the data
        throw new Exception('bad response from from API');
      }

      //check for 'problem_cause' element
      if(isset($this->xml->weather->problem_cause)){
        throw new Exception('API responded with error');
      }
    }
    return $this->xml;
  }

  public function getCondition()
  {
    //make sure there's data
    if(!isset($this->getXml()->weather->current_conditions)){
      //you could throw an exception and assume any code using this is 
      //calling it in a try block
      throw new Exception('could not find conditions');
    }

    return $this->getXml()->weather->current_conditions;
  }

  public function getTemp($type = 'f')
  {
    //validate type
    if(!in_array($type, array('f','c'))){
      throw Exception('invalid temp type: ' . $type);
    }

    $element = 'temp_' . $type;
    //make sure there's data
    if(!isset($this->getCondition()->{$element}['data'])){
      throw new Exception('could not find temp');
    }

    //cast as float and return
    return (float) $this->getCondition()->{$element}['data'];
  }
}