There are a couple of issues I see with the limited code you have provided.
Your House class has a public member type which means that at any point in the lifetime of the objects (instances of Houses) you can change the type. This makes your code not only hard to test, but also hard to maintain. Because the value of type cannot really be trusted (because it can be changed at any point). So the first thing I would do is make that property private. And set the property using the constructor of the class.
The second thing I notice is the isResources method which apparently does something with the database. But I don't see any database connection getting passed in. Neither in the constructor nor in the method. This is very suspicious, because that means the database connection is accessed by either:
- creating a new connection inside the method
- using some form of a global inside your method
Both have issue:
creating a new connection inside the method
This would mean you are tightly coupling the database connection to the House class with no easy (and sane) way to unit test your House. Because there is no way to swap the database connection with some other connection. Or even some totally other form of storage. Or perhaps some mocked storage.
Also this method would mean that you are going to have a lot of database connections throughout your application, because you are going to create a new connection in every class / method that needs it.
Also there is no way you can see by looking at the method signature you are actually using a database connection in there. This is called a hidden dependency and should be avoided whenever possible.
using some global inside your method
This has for most points raised the exact same problems as the above approach. Globals and global state should be avoided at all cost. Whether you are using the global keyword directly, access the $_GLOBALS array or whether you are using the singleton pattern. All have the same issues in regards to maintenance and testability.
I have already written down the reasons, drawbacks and solution for this in another post some while ago: Use global variables in a class
Another thing I notice in the isResources method is that based on the comment it checks for available resources. Now lets take that example into the real life. When you are going to build a house in real life do you really ask (or check) the house itself to see whether there are enough resources to build the house? No you don't. This is a violation of the Single Responsibility Principle and just doesn't make much sense (asking the house whether there are resources to build the house).
I see your class also has a buildHouse method, which is also something strange. Objects are constructed (build) using the constructor. There is no reason for that method to be there. You should pass all the information (elements of the house) into the constructor.
With the information I provided above (there is probably lots more I could tell you) you end up with something like the following:
<?php
class Factory
{
private $resources;
public function __construct(Resources $resources)
{
$this->resources = $resources;
}
public function build($type, array $coordinates)
{
if (!$this->resources->areAvailable()) {
throw new \UnavailableResourcesException('Not enough resources to build the house');
}
return new House($type, array $coordinates);
}
}
class Resources
{
private $dbConnection;
public function __construct(\PDO $dbConnection)
{
$this->dbConnection = $dbConnection;
}
public function areAvailable()
{
// check database for resources
return true;
}
}
class House
{
private $type;
private $coordinates;
public function __construct($type, array $coordinates)
{
$this->type = $type;
$this->coordinates = $coordinates;
}
}
$dbConnection = new PDO('mysql:dbname=yourdatabase;host=127.0.0.1;charset=utf8', 'user', 'pass');
$dbConnection->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$dbConnection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$resources = new Resources($dbConnection);
$factory = new Factory($resources);
$myHouse = $factory->build('tipi', array(22, 13));
Note there still can be enough improved in my example code above, but this is just to give you an idea to get started.
Also note that the advice given to you by fellow Stack Overflowers about looking into Yii, Cake or CI is terrible imho. Because those frameworks really do not teach good OOP practices at all. Yii for example is full of static method which basically means your application is full of global state. Cake isn't OOP by any definition of it. Also note that (again imho) Yii, CI and Cake are the three worst popular framework out there.
