patternphpMinor

Google Weather API wrapper

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

wrapperapigoogleweather

Problem

I'm an amateur programmer (self-taught). Anyway, this class provides a few methods to access Google's unofficial weather API. I'm having trouble on how to go about handling parsing errors in parse_xml() so any suggestions there would also be helpful.

```
_location = $location;

// urlencode doesn't seem to work so manually add the + for whitespace
$this->_url = preg_replace('/\s{1}/', '+',$this->_url .= $location);
$this->parse_xml($this->get_xml());
}

public function get_temp($type = "f") {
if (!$this->_isParsed)
return false;

// User specificed celsius, return celsius
if ($type == "c")
return $this->_wData['current']['temp_c'];

// return fahrenheit
return $this->_wData['current']['temp_f'];
}

public function get_condition() {
if (!$this->_isParsed)
return false;
// provide current conditions only
return $this->_wData['current']['condition'];
}

public function get_forecast_for_day($day) {
if (!$this->_isParsed)
return false;
return $this->_wData['forecast'][$day];
}
public function get_forecast_assoc() {
if (!$this->_isParsed)
return false;
return $this->_wData['forecast'];
}

public function get_cond_assoc() {
if (!$this->_isParsed)
return false;
return $this->_wData['current'];
}

public function dump_wData() {
if (!$this->_isParsed)
return false;
return $this->_wData;
}

public static function to_celsius($f) {
// Convert Fahrenheit to Celsius.
// I figured this would be quicker than trying to parse the XML.
return floor(((int)$f - 32) * (5 / 9));
}

private function get_xml() {
// Download raw XML to be parsed.
$ch = curl_init($this->_url);

// 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; r

Solution

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.

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

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.

Code Snippets

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

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

Context

StackExchange Code Review Q#4063, answer score: 3

Revisions (0)

No revisions yet.