Re: [Call for Vote] XML_Beautifier

Review: Reasonably deep Review, looked at source and examples Vote: +1 Account: alan_k Looks OK.. - tried to find things wrong with it.. very difficult... :) couple of little comments:: -------------------- -it would be nice would be a 'stategic overview of the method of operation at the top..' something like.. this works by parsing the xml into a series of tokens? - then goes throught the tokens (including blank space).. and outputs it... -------------------- - AFAIK theres nothing in pear rules about string concatination.. - but normally this

    $xml  = $indent."<?".$struct["target"].$this->_options["linebreak"];
    $xml .= $this->_indentTextBlock(rtrim($struct["data"]), $struct["depth"]);
    $xml .= $indent."?>".$this->_options["linebreak"];

would be slightly more effecient as

    $xml  = "{$indent}<?{$struct['target']}{$this->_options["linebreak"]}" .
     $this->_indentTextBlock(rtrim($struct["data"]), $struct["depth"]) .
    "{$indent}?>{$this->_options['linebreak']}";

or

    $xml  = $indent .

"<?" . $struct["target"] . $this->_options["linebreak"] . $this->_indentTextBlock(rtrim($struct["data"]), $struct["depth"]) . $indent . "?>" . $this->_options["linebreak"]; eg. no extra $xml .= 's and using {} or adding extra spaces or line breaks in between the . concat operator.. --------------------

        $struct    = array(
                         "type"    => XML_BEAUTIFIER_PI,
                         "target"  => $target,
                         "data"    => $data,
                         "depth"   => $this->_depth
                       );

        $this->_appendToParent($struct);

could just be..

    $this->_appendToParent(array(
                         "type"    => XML_BEAUTIFIER_PI,
                         "target"  => $target,
                         "data"    => $data,
                         "depth"   => $this->_depth
                       ));

of even easier..

    $type = XML_BEAUTIFIER_PI;
    $depth = $this->_depth;
    $this->_appendToParent(get_defined_vars());

-------------------- try using is_writeable(), rather than

        if( !$fp = fopen($newFile, "w")) {
            return $this->raiseError("Could not write to output file", XML_BEAUTIFIER_ERROR_NO_OUTPUT_FILE);
        }

which should really be $fp = fopen($newFile, "w"); if ( !$fp) { ... Try and avoid assignment and ! testing in the same equasion it looks like it could have unpredictable results.. (although it probably works..) -------------------- I would change the default behaviour of formatFile($filename) to return the formated string, rather than overwrite the file of the same name... - It's a little close to having a unpredictable behavour.. (eg. if you send it an empty string or something (eg. you mistype the tofile variable).. - it will overwrite, the file.. which may not be a desirable behavour... Regards Alan Stefan Neufeind wrote:

It looks nice and might be quite helpful - as Greg pointed out we might e.g. use it for formating the package.xml when building a package. +1 from my side. Review: Cursory review, looked at source and examples Vote: +1 Account: neufeind On 22 Sep 2003 at 10:21, Stephan Schmidt wrote:

last week I proposed a package I dubbed XML_Beautifier. After some short discussion, which ended in discussing a method called 'apiVersion()' instead of the package all people involved agreed on the name XML_Beautifier. So now I'd like to ask you to vote for this package if you want it included in PEAR. For those of you who missed the proposal, you can find sourcecode, docs, examples and an installable package at: http://www.php-tools.de/PEAR/XML_Beautifier/

-- Can you help out? Need Consulting Services or Know of a Job? http://www.akbkhome.com