Ping services Class -- Looking forward to your opinions

[LiamBailey]

Hi Guys,

I finally moved to a quality hosting company and am now on PHP5. I dived into learning classes, and I have written my first one. I am looking forward to hearing your thoughts on how you would have done it differently, so that I may learn more.

<?
class pinger {

protected $request;
protected $context;
protected $return;

protected $services = array(
'service1' => array('name' => "Technorati", 'url' => "http://rpc.technorati.com/rpc/ping/"),
'service2' => array('name' => "Feedburner", 'url' => "http://ping.feedburner.com/")
);

protected $method = "weblogUpdates.ping";
protected $parameters = array('Write About Property','http://www.write-about-property.com');

public function request_creator() {
return xmlrpc_encode_request($this->method,$this->parameters);
}

public function context_creator($request) {
return stream_context_create(array('http' => array(
    'method' => "POST",
		'header' => "Content-Length: 250",
		'header' => "Host:" . substr($service['url'],7,0),
		'header' => "User-Agent:LBaileysRPCPinger",
		 'header' => "Content-Type:text/xml",
    'content' => $request
		)));
}

public function __construct() {
$this->request = $this->request_creator();
$this->context = $this->context_creator($this->request);
}

public function ping($services) {
foreach ($services as $service) {
$this->file = file_get_contents($service[url], false, $this->context);
$this->response = xmlrpc_decode($this->file);
if (xmlrpc_is_fault($this->response)) {
    $return .= "Error with: " . $service['name'] . " error was: " . print_r($this->response[faultString] . $this->response[faultCode],true) . "<p/>";
} else {
    $return .= $service['name'] . " pinged. Response from server was: " . print_r($this->response,true) . "<p/>";
}
}
print_r($return);
}
}

$ping = new pinger();
$ping->ping($ping->services);

[LiamBailey]

Had to change the services var to public, so it can be passed into the ping function.

But I am considering making the services array a seperate file.

[NogDog]

LiamBailey;10912550 wrote:

Had to change the services var to public, so it can be passed into the ping function.

But I am considering making the services array a seperate file.

If you want that method to use the class variable $services, just use it internally (via $this) instead of passing it as a param:

public function ping() {
   foreach ($this->services as $service) {
      $this->file = file_get_contents($service[url], false, $this->context);
      $this->response = xmlrpc_decode($this->file);
      if (xmlrpc_is_fault($this->response)) {
         $return .= "Error with: " . $service['name'] . " error was: " . print_r($this->response[faultString] . $this->response[faultCode],true) . "<p/>";
      } else {
         $return .= $service['name'] . " pinged. Response from server was: " . print_r($this->response,true) . "<p/>";
      }
   }
   print_r($return);
}

[LiamBailey]

Yeah, that worked Nogdog, thanks.

Nothing else you would do differently from my class?

[NogDog]

I would probably try to make it more generic (and a bit more focused). Instead of hard-coding those various URLs and such, I would make them set-able by the client code, either via setter methods, as parameters of the constructor, or by making the variables public. If you want to ping two different URLs, then either you would instantiate a new object for each (particularly if you make the URL and related values into input parameters for the constructor) or else set them via setter methods (or directly if you want the variables to be public) before running the actual pinger method. Also, I would not output results directly from the class; leave that up to the client code which knows at any given time what it wants to do with the results. These changes would make the class more portable and reusable, rather than being tightly tied to the current application on which you are working right now.

PS: I'd format the code a bit more consistently, too, and add some phpDocumentor comments. :p

[LiamBailey]

Hi Nogdog,

Yes I know. By hardcoding it so, I have kind of defeated the purpose of writing it as a class.

But the thing is, it is only going to be implemented from one place on my site, so I could easily have done it without making it a class, but I only just finally learded

[NogDog]

LiamBailey;10912583 wrote:

Hi Nogdog,

Yes I know. By hardcoding it so, I have kind of defeated the purpose of writing it as a class.

But the thing is, it is only going to be implemented from one place on my site, so I could easily have done it without making it a class, but I only just finally learded

As you say, it might never be used anywhere else by you. Then again, you might find another use for it, or maybe a very similar use that just requires a few modifications. Or maybe the URLs will change in the future, etc. And anyway, if this is being done as a learning experience, it might as well be done in the spirit of learning how to use classes and objects in a larger application environment where modularity, loose-coupling, re-usability, and all those other catch-words come into play.

[LiamBailey]

It wasn't just for a learning experience. It is something that I needed to do, but instead of using a function or a page with query strings as I would have before, I decided to make it a class, because I had just learned them.

The reason most of the work is done within the class, is because of the script it is implemented in, which is very code heavy already.

It is a business I am running, so I don't have time for it to be just about the learning as it was primarily before. I needed a quick way to ping multiple services when i post and the class I created does that perfectly. If I want more services, i just add them to the array.

Your thoughts on the class are much appreciated though. They show that I have got the idea right. Originally a lot of the implementation was done outside the class itself, but I changed it so that implementing it was easier, because of the code heavy page it is implemented on. Thank you very much