PHP class library

Question

I am a PHP newbie, gone through a couple starter tutorials which went ok, so I thought I'd try creating a more complex project to get some experience. It is a NetBeans PHP project where I try to replicate some of the functionality from Microsoft's CodeDOM technology and Zend_CodeGenerator. Basically it is a class library that helps generating PHP code. My goals with this are:

Get accustomed to NetBeans
Get some routine in writing PHP code
Learn OOP
Learn about unit tests
Learn about versioning

I uploaded the project to GitHub (here). I would be thankful for any comments.

The base class:

<?php

/**
* @package UC_PHPCodeModel
* @author Uros Calakovic
*/

/**
* The CodeObject class is the base class for most code model objects.
*
* @abstract
*/

abstract class CodeObject
{
    public function __construct()
    {
        $this->Comments = new CommentStatementCollection();
    }

    /**
* __set() is used in CodeObject and inherited classes
* to make private / protected variables accessible from outside the class.
*
* __set() simply calls the appropriate field setter method.
* It is still possible to call field setter methods directly.
*
* @param string $name Name of the field to be accessed
* @param mixed $value The value the field should be set to, usually a collection.
*/

    public function __set($name, $value)
    {
        if($name == 'Comments')
        {
            $this->set_Comments($value);
        }
    }

    /**
* __get() is used in CodeObject and inherited classes
* to make private / protected variables accessible from outside the class.
*
* __get calls the appropriate field getter method.
* It is still possible to call field getter method directly.
*
* @param string $name Name of the field to be accessed
* @return mixed The field that $name specified, usually a collection
*/

    public function __get($name)
    {
        if($name == 'Comments')
        {
            return $this->get_Comments();
        }
    }

    /**
* The $UserData getter method
*
* @return mixed Returns any value stored in $UserData
*/

    public function get_UserData()
    {
        return $this->UserData;
    }

    /**
* The $UserData setter method
*
* @param mixed $UserData An arbitrary value
*/

    public function set_UserData($UserData)
    {
        $this->UserData = $UserData;
    }

    /**
* The $Comments getter method
*
* @return CommentStatementCollection
*/

    public function get_Comments()
    {
        return $this->Comments;
    }

    /**
* The $Comments setter method
*
* @param CommentStatementCollection $Comments
*/

    public function set_Comments(CommentStatementCollection $Comments)
    {
        $this->Comments = $Comments;
    }

    /**
* Stores the collection of comments for a code object.
*
* @var CommentStatementCollection
*/

    protected $Comments;

    /**
* Can contain arbitrary user-defined data related to a code object.
*
* @var mixed
*/

    protected $UserData = null;
}

?>

The child class representing a PHP statement:

<?php

include_once 'CodeObject.php';


/**
* This is a parent class for other statement classes
* and should not be instantiated.
*
* @package UC_PHPCodeModel
* @author Uros Calakovic
*/

class Statement extends CodeObject
{
    public function __construct()
    {
        parent::__construct();
    }

    public function __set($name, $value)
    {
        parent::__set($name, $value);

        if($name == 'BlankLinesAfter')
        {
            $this->set_BlankLinesAfter($value);
        }
    }

    public function __get($name)
    {
        $ret = parent::__get($name);

        if($ret != null)
        {
            return $ret;
        }

        if($name == 'BlankLinesAfter')
        {
            return $this->get_BlankLinesAfter();
        }
    }

    public function get_BlankLinesAfter()
    {
        return $this->BlankLinesAfter;
    }

    public function set_BlankLinesAfter($BlankLinesAfter)
    {
        if(is_int($BlankLinesAfter) )
        {
            if($BlankLinesAfter >= 0)
            {
                $this->BlankLinesAfter = $BlankLinesAfter;
            }
            else
            {
                throw new InvalidArgumentException(
                    'The value should be greater or equal to zero.');
            }
        }
        else
        {
            throw new InvalidArgumentException(
                'The value should be an integer value.');
        }
    }

    private $BlankLinesAfter = 0;
}

?>

The child class representing an if..else statement:

<?php

include_once 'ConditionStatement.php';
include_once 'StatementCollection.php';

/**
* Represents an if...else statement
*
* elseif is not supported
*
* @package UC_PHPCodeModel
* @author Uros Calakovic
* @example ../Classes/IfElseStatementExample.php
*/

class IfElseStatement extends ConditionStatement
{
    /**
* The IfElseStatement constructor
*
* Both $TrueStatements and $FalseStatements can be ommited by setting them to null
*
* @param Expression $Condition The boolean expression to be evalueted true or false
* @param StatementCollection $TrueStatements The statements in the if block
* @param StatementCollection $FalseStatements The statements in the else block
*/

    public function __construct(Expression $Condition,
        StatementCollection $TrueStatements = null,
        StatementCollection $FalseStatements = null)
    {
        parent::__construct();
        $this->set_Condition($Condition);

        $TrueStatements == null ?
            $this->TrueStatements = new StatementCollection() :
            $this->set_TrueStatements($TrueStatements);

        $FalseStatements == null ?
            $this->FalseStatements = new StatementCollection() :
            $this->set_FalseStatements($FalseStatements);
    }

    /**
* Used for making private / protected variables
* accessible from outside the class.
*
* @param string $name Name of the field to be accessed
* @param mixed $value The value the field should be set to, usually a collection.
* @see CodeObject::__set()
*/

    public function __set($name, $value)
    {
        parent::__set($name, $value);

        if($name == 'TrueStatements')
        {
            $this->set_TrueStatements($value);
        }

        if($name == 'FalseStatements')
        {
            $this->set_FalseStatements($value);
        }
    }

    /**
* Used for making private / protected variables
* accessible from outside the class.
*
* @param string $name Name of the field to be accessed
* @return mixed The field that $name specified, usually a collection
* @see CodeObject::__get()
*/

    public function __get($name)
    {
        $ret = parent::__get($name);

        if($ret != null)
        {
            return $ret;
        }

        if($name == 'TrueStatements')
        {
            return $this->get_TrueStatements();
        }

        if($name == 'FalseStatements')
        {
            return $this->get_FalseStatements();
        }
    }

    /**
* The $TrueStatements getter method
* @return StatementCollection
*/

    public function get_TrueStatements()
    {
        return $this->TrueStatements;
    }

    /**
* The $TrueStatements setter method
* @param StatementCollection $TrueStatements
*
* @assert (123) throws PHPUnit_Framework_Error
* @assert ('name') throws PHPUnit_Framework_Error
* @assert (new stdClass()) throws PHPUnit_Framework_Error
*/

    public function set_TrueStatements(StatementCollection $TrueStatements)
    {
        $this->TrueStatements = $TrueStatements;
    }

    /**
* The $FalseStatements getter method
* @return StatementCollection
*/

    public function get_FalseStatements()
    {
        return $this->FalseStatements;
    }

    /**
* The $FalseStatements setter method
* @param StatementCollection $FalseStatements
*
* @assert (123) throws PHPUnit_Framework_Error
* @assert ('name') throws PHPUnit_Framework_Error
* @assert (new stdClass()) throws PHPUnit_Framework_Error
*/

    public function set_FalseStatements(StatementCollection $FalseStatements)
    {
        $this->FalseStatements = $FalseStatements;
    }

    /**
* The statements in the if block
* @var StatementCollection
*/

    private $TrueStatements;

    /**
* The statements in the else block
* @var StatementCollection
*/

    private $FalseStatements;
}

?>

The test unit class for the if..else class:

<?php

require_once dirname(__FILE__) . '/../../Classes/IfElseStatement.php';

/**
* Test class for IfElseStatement.
* Generated by PHPUnit on 2012-05-23 at 21:05:00.
*/
class IfElseStatementTest extends PHPUnit_Framework_TestCase
{

    /**
* @var IfElseStatement
*/
    protected $object;

    /**
* Sets up the fixture, for example, opens a network connection.
* This method is called before a test is executed.
*/
    protected function setUp()
    {
        $this->object = new IfElseStatement(
            new BinaryOperatorExpression(
                    BinaryOperator::VALUE_EQUALITY,
                    new VariableReferenceExpression('a'),
                    new PrimitiveExpression(0)));
    }

    /**
* Tears down the fixture, for example, closes a network connection.
* This method is called after a test is executed.
*/
    protected function tearDown()
    {

    }

    /**
* Generated from @assert (123) throws PHPUnit_Framework_Error.
*
* @covers IfElseStatement::set_TrueStatements
* @expectedException PHPUnit_Framework_Error

*/
    public function testSet_TrueStatements()
    {
        $this->object->set_TrueStatements(123);
    }

    /**
* Generated from @assert ('name') throws PHPUnit_Framework_Error.
*
* @covers IfElseStatement::set_TrueStatements
* @expectedException PHPUnit_Framework_Error

*/
    public function testSet_TrueStatements2()
    {
        $this->object->set_TrueStatements('name');
    }

    /**
* Generated from @assert (new stdClass()) throws PHPUnit_Framework_Error.
*
* @covers IfElseStatement::set_TrueStatements
* @expectedException PHPUnit_Framework_Error

*/
    public function testSet_TrueStatements3()
    {
        $this->object->set_TrueStatements(new stdClass());
    }

    /**
* Generated from @assert (123) throws PHPUnit_Framework_Error.
*
* @covers IfElseStatement::set_FalseStatements
* @expectedException PHPUnit_Framework_Error

*/
    public function testSet_FalseStatements()
    {
        $this->object->set_FalseStatements(123);
    }

    /**
* Generated from @assert ('name') throws PHPUnit_Framework_Error.
*
* @covers IfElseStatement::set_FalseStatements
* @expectedException PHPUnit_Framework_Error

*/
    public function testSet_FalseStatements2()
    {
        $this->object->set_FalseStatements('name');
    }

    /**
* Generated from @assert (new stdClass()) throws PHPUnit_Framework_Error.
*
* @covers IfElseStatement::set_FalseStatements
* @expectedException PHPUnit_Framework_Error

*/
    public function testSet_FalseStatements3()
    {
        $this->object->set_FalseStatements(new stdClass());
    }

    /**
* @covers IfElseStatement::__set
*/
    public function test__set()
    {
        $trueStatements = new StatementCollection();
        $trueStatements->add(new AssignStatement(
            new VariableReferenceExpression('a'),
            new PrimitiveExpression(0)));

        $this->object->TrueStatements = $trueStatements;

        $this->assertTrue(
                $trueStatements === $this->object->get_TrueStatements());
    }

    /**
* @covers IfElseStatement::__set
*/
    public function test__set2()
    {
        $falseStatements = new StatementCollection();
        $falseStatements->add(new AssignStatement(
            new VariableReferenceExpression('a'),
            new PrimitiveExpression(0)));

        $this->object->FalseStatements = $falseStatements;

        $this->assertTrue(
                $falseStatements === $this->object->get_FalseStatements());
    }

    /**
* @covers IfElseStatement::__get
*/
    public function test__get()
    {
        $this->assertTrue(
            $this->object->TrueStatements === $this->object->get_TrueStatements());
    }

    /**
* @covers IfElseStatement::__get
*/
    public function test__get2()
    {
        $this->assertTrue(
            $this->object->FalseStatements === $this->object->get_FalseStatements());
    }

    /**
* @covers IfElseStatement::get_TrueStatements
*/
    public function testGet_TrueStatements()
    {
        $this->assertTrue(
            $this->object->get_TrueStatements() instanceof StatementCollection ||
            $this->object->get_TrueStatements() == null);
    }

    /**
* @covers IfElseStatement::get_FalseStatements
*/
    public function testGet_FalseStatements()
    {
        $this->assertTrue(
            $this->object->get_FalseStatements() instanceof StatementCollection ||
            $this->object->get_FalseStatements() == null);
    }

}

?>

The if..else class example usage:

<?php

include_once 'Classes/PHPCodeProvider.php';

/*
* Create an instance of the CodeGeneratorOptions class
* using the default options for code generation.
*/

$options = new CodeGeneratorOptions();

/*
* Create an instance of the IndentedTextWriter.
* After the generateCodeFromStatement call
* $writer will hold the generated code.
*/

$writer = new IndentedTextWriter();

/*
* Create an instance of the PHPCodeProvider class
*/

$provider = new PHPCodeProvider();

/*
* Variable and field references that will be used
* to generate if statements
*/

$thisReference = new ThisReferenceExpression();
$isWinning = new VariableReferenceExpression('isWinning');
$attemptNumber = new FieldReferenceExpression('attemptNumber', $thisReference);
$gameFinished = new FieldReferenceExpression('gameFinished', $thisReference);

/*
* A simple if statement
*/

$ifElseStatement1 = new IfElseStatement($isWinning);

$ifElseStatement1->TrueStatements->add(
    new AssignStatement(
        new FieldReferenceExpression('isWinning', $thisReference),
        $isWinning));


/*
* Generate the code and output the result.
*/

$provider->generateCodeFromStatement($ifElseStatement1, $writer, $options);

echo
    '<pre>' . str_replace(
        '<br />', '', highlight_string($writer->get_String(), true)) .
    '<pre/>';

$writer->clear();

/*
* An if statement with both the if and the else block
*/

$ifElseStatement2 = new IfElseStatement(
    new BinaryOperatorExpression(
        BinaryOperator::LESS_THAN,
        $attemptNumber,
        new PrimitiveExpression(8)));

$ifElseStatement2->TrueStatements->add(
    new AssignStatement(
        $attemptNumber,
        new PrimitiveExpression(1),
        CompoundAssignmentOperator::PLUS_ASSIGN));

$ifElseStatement2->FalseStatements->add(
    new AssignStatement(
        $gameFinished,
        new PrimitiveExpression(true)));

/*
* Generate the code and output the result.
*/

$provider->generateCodeFromStatement($ifElseStatement2, $writer, $options);

echo
    '<pre>' . str_replace(
        '<br />', '', highlight_string($writer->get_String(), true)) .
    '<pre/>';

/*
* The output should be:
*
* if($isWinning)
* {
* $this->isWinning = $isWinning;
* }
*
*
* if($this->attemptNumber < 8)
* {
* $this->attemptNumber += 1;
* }
* else
* {
* $this->gameFinished = true;
* }
*
*/

?>

Please post your code here and specify what kind of feedback you're looking for: code correctness, best practices and design pattern usage, application UI, security issues, etc.
Thanks for the comment. I have ~70 PHP classes, too much code long to post here. If providing only a link is not acceptable, I'm sorry, somebody close the topic. I would like feedback on unit tests, project OOP design (inheritance, etc) and my code in general as I am new to PHP.
Maybe try to find a class or two that's representative of your coding style, and post it here.

braunbaer · Accepted Answer · 2012-06-07 20:07:32Z

as i do not know the technologies you try to implement i will focus on general stuff.

first:

public function __construct()
{
    $this->Comments = new CommentStatementCollection();
}

it would be better to inject "CommentStatementCollection" into the constructor

public function __construct(Collection $CommentCollection)
{
    $this->Comments = $CommentCollection
}

this has some benefits one of them is better unit testing as you are now able to mock the collection. CommentStatementCollection was only an example, this advice is true for every "new" statement within your classes.

   public function __construct()
    {
        parent::__construct();
    }

if you do not extend the behavior of the parent constructor there is no point in overriding it to call it again.

+++ EDIT

As for calling parent constructor, I need it to initialize my $Comments property in the child class, I thought this would not happen if I didn't call the parent constructor explicitly?

here some test code

<?php
abstract class test {
    public function __construct() {
        $this->test = 'test';
    }
}
class child extends test {

}

$test = new child();
var_dump($test->test); // string(4) "test"

--- EDIT

i would suggest not to use the magic methods __set and __get as they obfuscate the code and you use custom setter anyway (like set_UserData)

new PrimitiveExpression(8)));

there you have magic numbers

you could use __toString instead of implementing "get_string", but there is nothin wrong if that.

testSet_TrueStatements2

this is very bad naming of tests. If they fail you will not know why they failed or what failed at first glance. Try to use names like "testSet_TrueStatementsWithInvalidArgument" or something like that, do not worry about long method names! Also you could work with dataproviders to test the same functionality with different arguments.

    $trueStatements = new StatementCollection();
    $trueStatements->add(new AssignStatement(
        new VariableReferenceExpression('a'),
        new PrimitiveExpression(0)));

This looks like you can use a Builder or Factory class to seperate the setup of your objects.

+++ EDIT2

class StatementFactory {
  public static function getAssignStatement($var, $prim) {
    return new AssignStatement(new VariableReferenceExpression($var), new PrimitiveExpression($prim));
  }
}
//...
        $trueStatements = new StatementCollection();
        $trueStatements->add(StatementFactory::getAssignStatement('a', 0));

i would have to dive deeper into your code if this factory makes sense in the context of your implementation but maybe you get the idea. I dont think it is a good factory, just an example.

--- EDIT2

I think thats all on the first look ( - : hope it helps

Good points. Regarding the first issue, I've been thinking to refactor all the constructors so that the arguments have type hints and default value of null. This way I would need to initialize my variables only if nothing is passed to the constructor. As for calling parent constructor, I need it to initialize my $Comments property in the child class, I thought this would not happen if I didn't call the parent constructor explicitly? Your point about __get and __set is actually one of my biggest dilemmas, I only added that so I could access fields like $var->Comments instead of using getters.
I only did because I prefer to use $var->Comments instead of $var->getComments(), but I've read that __set() and __get() are slow, so I may remove them after all.The tests were generated by NetBeans, never thought of changing method names. If you mean $writer->get_String() this is just a getter for a private field that contains the generated code and I will need to change the name to something meaningful first thing.
One thing I'm not clear though is your last comment. I realize that code looks messy but I don't know how to make it more easier to use. $trueStatements is an array of Statement objects. Each Statement object has its own constructor so I ended up with a new statement within a new statement.I'v read something about the Factory pattern but I'm not sure how to apply it here. Thanks again for the comments, you've nailed several things I've been thinking about.
@Anonimista as for the constructor issue i extended my post by an example. As for the other questions i will have to answer them later this day

mseancole · Answer 2 · 2012-06-07 17:08:05Z

Again braunbaer beat me to it. This is just an addition to what he already has. +1 braunbaer :)

NetBeans

Its a good IDE, but like most free things there are bugs with it. I'm using it currently, so don't think I'm trying to persuade you to choose another IDE. Just thought I'd point out a few issues I've been having so you don't spend hours trying to figure out why something isn't working right. Line wrapping doesn't always work (more noticeable with HTML, my PHP isn't that long). I've been noticing recently that PHPDoc comments once edited cannot be collapsed again until the page is reloaded. Its really slow at loading dirty XML (no indents), and can actually lag the entire program while they are up, clean XML seems to work fine. There are a few others, but I can't remember them all off the top of my head. Again, this is just a heads up not a warning against it. I've yet to find a better (free) IDE.

Learn OOP

I tell this to everyone who says they are new to PHP. Learn PHP first! You don't want to start learning OOP until you are good and familiar with the language, any language. Its like trying to read fine literature when you're still trying to sound out your words. Sure you can do it, but it will be slow and cumbersome. Save yourself a lot of headaches and learn the basics first. The only time I'd suggest jumping right into OOP is if you are coming from another language and feel comfortable in converting that knowledge into PHP knowledge. OOP is a completely different beast and should be tackled when you're not still struggling with the basics. Its good to push your self with difficult projects like this, but I think you bit off more than you could chew for your first project.

That was before I actually started reading your code. From what I can see, you seem versed enough to start looking into it. The only reason I'm leaving the above is as a disclaimer :)

Setters/Getters

I agree with braunbaer, I don't think it a good idea to use the magic setter and getter methods. At least not in a "public" class. I think the only "good" reason to use them is in factory classes in which it is all done behind the scenes and on variables that have already been verified and sanitized. But I've not gotten into that yet so I can't tell you much about it. Also as braunbaer started to say but did not elaborate on. Your magic methods are not truly doing anything that another method does not already do (set_Comments()). A true magic setter would use variable variables (read: evil) or a "master" array to accomplish what you are trying and would do so for anything passed to it, or for a select few "allowed" variables. Maybe there are other ways, but I tend to steer clear of this kind of thing so I'm unfamiliar with them. Now, I, and I'm sure everyone else here, am going to hate myself for showing you this, but here's the "proper" way to do what you are attempting.

public function __set( $name, $value ) {
    $method = "set_$name";
    if( method_exists( $this, $method ) { $this->$method( $value ); }

    //Here are those other ways I was talking about...
    if( in_array( $name, $this->allowedVariables ) ) {
        $this->$name = $value;
        //OR
        $this->_data[ $name ] = $value;
    }
}

However, you should just cut out the middle man (setter/getter) and just call the method you've already created for that purpose manually. Sure you can't do cool things like $var->Comments = 'blah'; anymore, but it is more secure and easier to debug.

include_once

Try not to use (include/require)_once. They pretty much say, "I'm not sure if I've already included this, so please make sure I don't do so again." You should be sure. And not that it will make much difference here, but if you ever find yourself including a bunch of files you will notice a drop in speed because of it.

So Many Classes

Unless you are planning on extending these later I don't see the point in having a class who's only purpose is to set and get a variable. I'd seriously consider a factory class to do this.

Ternary Operations

Ternary operations are meant to improve your code by shortening it. Many argue that they are bad form, but I am not one of those people (within limits). My only necessity for ternary expressions is that they be short and legible, and never extend more than one line. I don't think they should be used to replace if/else statements, so what you have in your ConditionStatement constructor looks off to me. I would rewrite it like so...

if( ! $TrueStatements ) { $TrueStatements = new StatementCollection(); }
$this->set_TrueStatements( $TrueStatements );

Thank you for the comments. I've been only using NetBeans for a couple of months for this single project. The good thing is that it helped me to start with PHPDoc, PHPUnit and GIT. I've recently started using these from the command line (I'm on Windows) which is probably more difficult for a beginner but the IDE helped me with the basics.
I know programming basics (control structures, data types etc) even some OOP concepts (I think I do). I've read through a couple of PHP basics tutorials. I started this as a learning experience so I don't mind that this project is too much for me (and it is). I feel a lot more comfortable with PHP than I did a month ago.
You're right those __set() and __get() methods don't really add anything useful, I only added them because I like to use $var->Comments->add() instead of $var->_getComments()->add, but I think I can safely remove them.
I noticed that my include_once statements slow down things when using the debugger. PHP jumps to the file it is supposed to be including, says 'I know you already' and jumps back. That's one more thing to look into.
You're right about too many classes. When I started this, I followed Microsoft's CodeDOM implementation - they have a separate class for almost every language construct that do nothing more than contain relevant variables. There is also one behemoth class (PHPCodeGenerator in my case) that does all the code generation. I have ~1300 lines of code in it right now, but I am thinking of getting rid of it and let each class generate it's own code (so that IfElseStatement contains the code that generate PHP if..else statements etc)

asked	11 months ago
viewed	184 times
active	11 months ago

PHP class library

2 Answers

Your Answer

Not the answer you're looking for? Browse other questions tagged php oop unit-testing or ask your own question.

PHP class library

2 Answers

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged php oop unit-testing or ask your own question.

Related