XML parser using PHP

It is just one of the files. I have also tried to write some tests using PHPUnit. Please give me some suggestions to improve my coding-writing skills.

The below is the test file for the above file:

```
current_line = "";
return $objectParseXML->current_line;
}

/**
* @depends testSetup
* @depends testParseObject
*/
public function testtagNameOn($objectParseXML, $currentLine){
$this->assertEmpty($objectParseXML->tree);
$objectParseXML->scanCharacter('assertTrue($objectParseXML->isTagName);
}

/**
* @depends testSetup
* @depends testParseObject
*/
public function testTagContentOn($objectParseXML){
$objectParseXML->isTagName = True;
$objectParseXML->scanCharacter('>', 3);
$this->assertTrue($objectParseXML->isTagContent);
$this->assertNotTrue($objectParseXML->isTagName);
}

}

?>

Code Files:

fileHandler = @fopen($filename, $mode);
}
public function getHandler()
{
return $this->fileHandler;
}
public function getHandlerType(){
return get_resource_type($this->fileHandler);
}
}
?>

isTagContent){
$this->isTagContent = False;
}
if($index+1 getLen())
{
$this->isTagName = $this->tagNameOn($this->current_line[$index+1]);
}
}
elseif($this->isTagName and (strcmp($char, '>')==0))
{
$this->isTagName = $this->tagNameOff();
$this->isTagContent = $this->tagContentOn();
}
elseif((strcmp($char, '/')==0)){
$this->isTagName = False;
$this->isTagContent = False;
}
elseif($this->isTagName and !strcmp($char, ' '))

Good to see you are using PHPUnit as testing method :)

Issuelist:

• Curly brackets are sometimes in same line as a method header and sometimes in the following

• There are both empty methods and methods that return always true/false

• Not every method has a self-describing name

• There are global attributes with the access modifier public

• There are multiple returns in one method

• Classes are included manually

• PHP-tags are closed

Recommendation

Curly brackets are sometimes in same line as a method header and sometimes in the following

For the sake of a good code-reading and understanding code should be structured. Write curly brackets either in the same line as the method header or in the following.

There are both empty methods and methods that return always true

ParseXML::goOverLine(): When a method body is empty it usually means you have not implemented its logic yet. It is possible that one forget to include its logic but calls the method which can leads to a difficult to identify bug at a later time. Therefor I recommend to throw an Exception with message Method not implemented.

ParseXML::tagContentOn(), ParseXML::tagNameOff(): Why does this method returns true/false anytime? Does it switches tags on/off?

ParseXML::gatherTagName($char), ParseXML::gatherTagContent($char): These methods rather append, do they? Or does this word describes appending as well?

Not every method has a self-describing name

ParseXML::getLen(): What does it return? Object Length, Current Line Length, ...? I recommend to rename the method to what it does - getCurrentLineLength().

There are global attributes with the access modifier public

An object is responsible for its valditity. As of that its attributes has to be setted via setters always and the attributes has to have as access modifier either protected or private.

There are multiple returns in one method

This makes the code less maintainable. Having more than one return means there are multiple scenarios when the method can be stopped. In case of a bug one need to debug through the whole method to figure out the return-point.

Classes are included manually

Instead of including classes manually it is recommended to make usage of the autoloader function (http://php.net/manual/en/function.spl-autoload-register.php). The advantage is that you don't have to worry about including a class.

PHP-tags are closed

It is not recommend to close the PHP-tag. It can happen that you have an empty space after the closed PHP-tag which leads to headers already sent error. Not closing them reduces headaches :)

StackExchange Code Review Q#121003, answer score: 3