Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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();
}

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

And 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 everytime (which I solved now using : as a delimiter)

I hope you guys can check it out for me :)

Edit: Can I bump? :P Bump v2 Bumping again because I want more critic~

share|improve this question
 
Just out of curiosity, in your execution script (that last bit), why did you close and open your script to output a one line string? Why not just echo 'SSL needs to be active';? BTW, this isn't a critique - I'm just curious. –  65Fbef05 Mar 23 '11 at 14:30
 
I've been flamed/bashed a lot on the subject of echo'ing stuff. Echo'ing HTML is generally bad. Not that it matters that much in this example because its just a test.php to test my functions. –  Jeffro Mar 24 '11 at 7:38
 
Really? echo just returns to stdout - I can't imagine why someone would get onto you about your output method (especially in a case like this). Now... If you're output is multiple lines, it is good practice to use an output buffer - but then you would echo the resulting var. I would like to hear someone's case against echo. Clearly it's there for a reason, and that reason is output. –  65Fbef05 Mar 24 '11 at 9:30
add comment

1 Answer

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

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.