5
\$\begingroup\$

I've tried to write an "OOP" approach to printing a website using the DOMDocument class. I'm kind of stuck here because I feel like I am blundering in how I am passing and using the DOMDocument. I found that I can just return the object from the class and use it that way.

What are some safe practices here? Best approach? I know it's far from OOP, but I at least want to be considerate of the model while learning. I realize my approach is probably all backwards. How bad is this code and what are some ways to improve this?

All styling is done in CSS, any and all critique is welcome about any aspect of the code.

If this isn't considered to be a useful question let me know and I will remove it.

<?php
require 'viewer.php'
//new viewer class
$v= new viewer();
//read html document structure from a template
$page= $v->setPage('template.html');
//find body
$body= $v->returnBody();
//create new element and attach it to body
$v->newElement('div', $body, "id=container");

$cont = $v->getElement("container");

$v->newElement('div', $cont, "id=title", "My Website");
$v->newElement('ul', $cont, "id=menu;class=menu");
$v->newElement('li', $v->getElement("menu"), NULL, "Forums" );

$v->newElement('div', $cont, "id=content");
$v->newElement('div', $v->getElement("content"), "id=content_title", "Content Title Here!");
$v->newElement('div', $v->getElement("content"), "id=content_box");

$mb = $v->getElement("content_box");
$v->newElement('a',$mb,NULL,"Some Link");

$content = " And after the link, this is the content.";

$dd=$v->rDOM();
$c=$dd->createTextNode($content);
$mb->appendChild($c);
$v->printHTML();
?>

Where the Viewer class is:

<?php
class viewer {
    protected $dom;

    public function __construct(){
        $this->dom = new DOMDocument();
    } 

    //Creates an element of the type passed in string, appends parameters passed in string
    //and finally inserts into the passed node
    public function newElement($type, $insertInto = NULL, $params=NULL, $content=""){
        $element = $this->dom->createElement($type, $content);
        if(gettype($params) == "string" && strlen($params) > 0){
            $attrCollec = preg_split("/;/", $params);
            foreach($attrCollec as $attr){
                $value = preg_split("/=/", $attr);
                $element->setAttribute($value[0], $value[1]);
            }
        }
        $insertInto->appendChild($element);
    }
    public function getElement($param){
        //if($this->dom->validate()){
            $elem = $this->dom->getElementById($param);

            return $elem;
        //}
    }
    public function setPage($fileName){
        //does this even work? should validate before each getElement call?
        $this->dom->validateOnParse = true;
        $this->dom->loadHTMLFile($fileName);            
    }        
    public function printHTML(){

        echo $this->dom->saveHTML();
    }
    public function returnBody(){
        $nodeList = $this->dom->getElementsByTagName('body');
        $body = $nodeList->item(0);

        return $body;
    }
    public function rDOM(){
        return $this->dom;
    }
}
?>
\$\endgroup\$
4
\$\begingroup\$

Some things I spot on the fly:

  • Lack of documentation. PHPDoc works well, every method, every class, every file should have a DocBlock.
  • Naming conventions. It's considered good naming convention to have methodNames() and $variableNames in camelCase, but ClassNames and ObjectNames in UppercaseCamelCase.
  • Code style. Conforming to a single code style is great. Conforming to an established standard is awesome!. Check out the PSR Code style standards page.
  • On the newElement method, preg_split is redundant. explode by ; or = is enough, and is faster!
  • Last but not least, if the entire file is PHP (without embedding HTML), it's considered safe (and even recommended!) to omit the closing ?> at the end of the file. This helps with line break and whitespace issues.

I'll continue editing as I explore more.

\$\endgroup\$
1
\$\begingroup\$

You should consider to use an array to pass the attributes to the newElement method. Also you need a DOMElement as parameter, as you use call the method appendChild, but you made it optional.

public function newElement($type, DOMElement $insertInto, $params=array(), $content=""){
    $element = $this->dom->createElement($type, $content);
    foreach($params as $attr => $value){
        $element->setAttribute($attr, $value);
    }
    $insertInto->appendChild($element);
}

Usage:

$v->newElement('div', $body, array('id'=>"container"));

In my oppinion it will become very hard to read and handle if you want to create a complex pages with tables and forms with the DOM class. Personally I would prefer a plain PHP template with placeholders or a template engine like smarty or twig for example.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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