Generating code with XML

Question

I'm working on my graduate project and I had stumbled upon a somewhat dilemma, I've managed to solve it with some workarounds but I have my doubts that this is the most efficient way to deal with this problem.

I'm writing a class to deal with the Plesk API and making it as flexible as possible for easy use.

Here is the function for generating the XML that will be send out as request:

private function emailCreatePacket($domain, $params){
    // Create new XML document.
    $xml = new DomDocument('1.0', 'UTF-8');
    $xml->formatOutput = true;

    // Create packet
    $packet = $xml->createElement('packet');
    $packet->setAttribute('version', '1.4.2.0');
    $xml->appendChild($packet);
    $mail = $xml->createElement('mail');
    $packet->appendChild($mail);
    $create = $xml->createElement('create');
    $mail->appendChild($create);
    $filter = $xml->createElement('filter');
    $create->appendChild($filter);
    $domain_id = $xml->createElement('domain_id', $domain->id);
    $filter->appendChild($domain_id);
    $mailname = $xml->createElement('mailname');
    $filter->appendChild($mailname);
    foreach ($params as $key => $value) { 
        $node = $mailname;
        if(strpos($key, ':') !== false){
            $split = explode(':', $key);
            $key = $split[1];
            $node = ${$split[0]};
            if(!isset(${$split[0]})){
                ${$split[0]} = $xml->createElement($split[0]);
                $mailname->appendChild(${$split[0]});
            }
            $xmlelement = $xml->createElement($key, $value);
            ${$split[0]}->appendChild($xmlelement);
        }else{
            $xmlelement = $xml->createElement($key, $value); 
            $node->appendChild($xmlelement);
        }
    }
    return $xml->saveXML();
}

Here's my public function:

public function createEmail($host, $domain, $params){
    $curl = $this->curlInit($host);
    $packet = $this->emailCreatePacket($domain, $params);
    echo $packet;
    $response = $this->sendRequest($curl, $packet);
    echo $response;
    if($this->parseResponse($response)){
        return true;
    }
}

Here are the functions called:

private function curlInit($host)
    {
        $security = new Security();
        $password = $security->decode($host['host_pass']);
        $curl = curl_init();
        curl_setopt($curl, CURLOPT_URL, "https://{$host['host_address']}:{$host['host_port']}/{$host['host_path']}");
        curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
        curl_setopt($curl, CURLOPT_POST,           true);
        curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, false);
        curl_setopt($curl, CURLOPT_SSL_VERIFYHOST, false);    
        curl_setopt($curl, CURLOPT_HTTPHEADER,
             array("HTTP_AUTH_LOGIN: {$host['host_user']}",
                    "HTTP_AUTH_PASSWD: {$password}",
                    "HTTP_PRETTY_PRINT: TRUE",
                    "Content-Type: text/xml")
        );    
        return $curl;
    }

private function sendRequest($curl, $packet){
        curl_setopt($curl, CURLOPT_POSTFIELDS, $packet);
        $result = curl_exec($curl);
        if (curl_errno($curl)) {
            $error  = curl_error($curl);
            $errorcode = curl_errno($curl);
            curl_close($curl);
            throw new Exception("Er is iets mis gegaan: <br /><br /> Foutcode: " . $errorcode . "<br /> Foutmelding: " . $error );
        }
        curl_close($curl);
        return $result;
    }

private function parseResponse($response){
    $xml = new SimpleXMLElement($response);
    $status = $xml->xpath('//status');
    if($status[0] == "error"){
        $errorcode = $xml->xpath('//errcode');
        $errortext = $xml->xpath('//errtext');
        throw new Exception("Er is iets mis gegaan: <br /><br /> Foutcode: ". $errorcode[0] . "<br /> Foutmelding: " . $errortext[0]);
    }else{
        return $response;
    }
}

Here is the code I use to call my function:

<?php

/**
 * @author Jeffro
 * @copyright 2011
 */

require('init.php');

$security = new Security();

if($security->sslactive()){
    try
    {
        $hosts = new Hosts();
        $host = $hosts->getHost("192.168.1.60");
        $pleskapi = new PleskApi();
        $client = $pleskapi->getClientInfo($host, 'janbham');
        $domain = $pleskapi->getDomainInfo($host, "lol.nl");
    $params = array('name' => 'ohecht', 'mailbox:enabled' => 'true', 'mailbox:quota' => '100000000', 'alias' => 'aapjesrollen', 'password' => 'testmail', 'permissions:manage_drweb' => 'true');
    $pleskapi->createEmail($host, $domain, $params);
    }
    catch (Exception $ex)
    {
        echo $ex->GetMessage();
    }
}else{
    ?>
    SSL needs to be active
    <?php
}

?>

The problem was regarding the structure the API uses with it deeper nesting every time (which I solved now using ':' as a delimiter)

Answer 1

Generally speaking, I like your code. I think it's well planned with few inefficiencies. I'm also impressed by your use of PHP's DOMDocument class. When creating XML via PHP, I usually opt for foreach loops and output buffering. I've been aware of this object class, but your example is the first I've ever seen it put to use. +1 for showing me something new! :D

Answer 2

• In createEmail you only return true in one case; I'd just return the value of parseResponse directly, then again, you don't seem to use the return value.
• Being nitpicky, the style is a bit inconsistent, that includes spacing, quotes, but otherwise looks good. The $params creation should be on multiple lines though, IMO 200 characters kind of stretches it.
• You could often leave out the last else block when you've already thrown an exception in the true case.
• What's with the random echo calls?

Now the XML creation is, let's say, a bit verbose. emailCreatePacket should use at least a helper function, or, if you have a recent enough version of PHP I understand that you could also use a local anonymous function instead. So for starters, how about something like this:

private function addChild($xml, $parent, name) {
    $child = $xml->createElement(name);
    $parent->appendChild(child);
    return $child;
}

private function emailCreatePacket($domain, $params){
    $xml = new DomDocument('1.0', 'UTF-8');
    $xml->formatOutput = true;

    $packet = addChild($xml, $xml, 'packet');
    $packet->setAttribute('version', '1.4.2.0');

    $mail = addChild($xml, $packet, 'mail');

    $create = addChild($xml, $mail, 'create');
    ...
}

You get the idea. Also check the PHP documentation, I think you'd be faster with using SimpleXML here as well, unless I'm missing a reason why you're using the DOM instead. Thirdly, I would think about using a separate PHP file to generate XML instead, after all, you're already basically using a templating engine.

The splitting of the parameters is probably fine, except I don't understand why you'd need that instead of passing nested arrays? Also the current code there is limited to two levels, right, so for extra points make that usable with arbitrarily nested parameters.