开发者

PHP Class improvements

I've made my first class using TDD (SimpleTest). It's working pretty well. This class parses an XML config file and returns it as an array. How could I improve it (performance, any tips)? What about the class responsabilities? Maybe XMLtoArray should be moved to another class, i don't know...

<?php
class Configuration
{
    private $domdocument_object;
    private $domxpath_object;

    public function __construct($filename) {
        $this->loadXML($filename);
        $this->domxpath_object = new DOMXPath($this->domdocument_object);
    }

    private function loadXML($filename)
    {
        if (!file_exists($filename))
        {
            throw new ConfigurationException('Configuration file not found');
        }

        $this->domdocument_object = $domdocument_object = new DOMDocument();
        $this->domdocument_object->preserveWhiteSpace = false;

        if (!$this->domdocument_object->load($filename))
        {
            throw new ConfigurationException('Malformed configuration file');
        }
    }

    public function get($path = '/*') {
        $configuration = array();

        $domnodelist_object = $this->domxpath_object->query($path);
        $configuration = $this->XMLToArray($domnodelist_object);

        /**
         * Ge开发者_StackOverflow中文版t a configuration entry as string or array
         *
         * For example:
         * $xml = '<foo><bar>baz</bar></foo>'
         * $path = '/foo/bar/'
         * return just baz, as string instead of an array('baz');
         *
         * Another example:
         * $xml = '<foo><bar>baz</bar><lorem>ipsum</lorem></foo>';
         * $path = '/foo'
         * return just array('bar' => 'baz', 'lorem' => ipsum);
         * instead of array('foo' => array('bar' => 'baz', 'lorem' => ipsum));
         */
        while (!is_string($configuration) && count($configuration) == 1)
        {
            $configuration_values = array_values($configuration);
            $configuration = $configuration_values[0];
        }

        if (empty($configuration))
        {
            $configuration = null;
        }

        return $configuration;
    }

    public function XMLToArray(DOMNodeList $domnodelist_object) {
        $configuration = array();

        foreach ($domnodelist_object as $element)
        {
            if ($element->nodeType == XML_DOCUMENT_NODE)
            {
                if ($element->hasChildNodes())
                {
                    $configuration = $this->XMLToArray($element->childNodes);
                }
            }
            else if ($element->nodeType == XML_ELEMENT_NODE)
            {
                if (!$element->hasChildNodes())
                {
                    $configuration[$element->nodeName] = null;
                }
                else if (
                    $element->firstChild->nodeType == XML_TEXT_NODE ||
                    $element->firstChild->nodeType == XML_CDATA_SECTION_NODE
                )
                {
                    $configuration[$element->nodeName] = $element->nodeValue;
                }
                else if ($element->firstChild->nodeType == XML_ELEMENT_NODE)
                {
                    $configuration[$element->nodeName] = $this->XMLToArray($element->childNodes);
                }
            }
        }

        return $configuration;
    }
}
?>

This class ignores XML attributes. Thank you.


Something that stood out to me is your creating a new object every time an object is executed, you should store the object locally (in the object) and then it only uses 1 portion of your memory.

Here are the changes I would do:

class Configuration
{
    private $domdocument_object;
    private $domxpath_object; //+

    public function __construct($filename)
    {
        $this->loadXML($filename);
        $this->domxpath_object = new DOMXPath($this->domdocument_object); //+
    }

    public function get($path = '/*')
    {
        //Remove the following
        $domxpath_object = new DOMXPath($this->domdocument_object);
    }
}

and then change the $domxpath_object to $this->domxpath_object where its required.

But this should really be moved over to CoderReview as this is off topic.


This is bad for performance:

$xml = preg_replace("/>\s+</", "><", $xml);

Aditionally it is not guaranteed to be reliable (this could change comments and CDATA-sections in undesired ways). It is though not easy to find a better solution. Iterating over all text nodes trimming them will be more reliable, but not faster.

If you just care for making this an array php's SAX parser or SimpleXML may be more suitable. Both options may be faster (I did not test).

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜