Basic "bee game" labeled as messy

Could you tell me how to make it quality code?

```
';
}
public function start($msg=null) {
unset($_SESSION['bees']);
$bee = new Bee();
$bee->setType('queen');
$this->addBee($bee,1);
$bee->setType('worker');
$this->addBee($bee,5);
$bee->setType('drone');
$this->addBee($bee,8);
if($msg)
return $msg;
else {
var_dump($_SESSION['bees']); // debug
return 'Game started ';
}
}
public function hit() {
$msg = '';
if($_SESSION['bees'])
$rd = array_rand($_SESSION['bees'],1);
else
$this->start('Game will start again');
$_SESSION['bees'][$rd]['lifespan'] = $_SESSION['bees'][$rd]['lifespan'] - $_SESSION['bees'][$rd]['hit'];
if($_SESSION['bees'][$rd]['lifespan']start('Game start again, the Queen is dead.'); break;
default: $msg .= 'One \'' . $rd . '\' is gone';
}
}
if($_SESSION['bees'][$rd]['bees'] ' . $msg;
}
public function addBee(Bee $newbee, $number = 1) {
$tipo = $newbee->getType();
$bee = $newbee->get();
$_SESSION['bees'][$tipo] = $bee;
$_SESSION['bees'][$tipo]['bees'] = $number;
$_SESSION['bees'][$tipo]['life'] = $newbee->getLifespan($tipo);
}
}

class Bee
{
protected $_type = null;
protected $_types = array(
'queen' => array(
'hit' => 8,
'lifespan' => 100
),
'worker' => array(
'hit' => 10,
'lifespan' => 75
),
'drone' => array(
hit' => 12,
'lifespan' => 50
)
);
public function setHit(){}
public function setLifespan(){}
public function addNewType(){}
protected function _set_hit(){}
protected function _set_lifespan(){}
protected function _add_new_type(){}
public function getLifespan($type){
if(array_key_exists($type,$this->_types))
return $this->_types[$this->_type]['lifespan'

One big issue with this code is that you've got a tight coupling between the business logic and the presentation. For example, the method play() really jumps out:

public function play(){

    return '';

}

From its name I'd think that this method plays something (a game? A piece of music? A role in a play? Another issue is clear, unambiguous naming of classes, methods and variables). I wouldn't expect it to output the HTML markup for a button with a JavaScript event attached to it.

This might not seem like a big deal, and in this particular case it probably isn't. But ask yourself this, how much work would you need to do if whoever set you this project decided half way though that it should run through the command line instead of a web browser?

Ideally, your PHP classes should contain no presentation logic at all. They should return data structures (arrays, scalar types, simple data transfer objects, etc) to the outside world, and then a separate process should take that information and render it to the user in a suitable presentation. The classes that do the actual work (the business objects) should implement all the logic and rules needed to accomplish a task without any concern as to how the results of that task are presented.

This concept is known as separation of concerns. A concern is a particular part of the software package you're developing which shouldn't be tightly bound (coupled in the lingo) to other parts of your package (sorry I can't really word it better than that). Implementing the rules and maintaining the state of the game is one concern, getting user input and displaying output to the user is a different concern. While there obviously has to be some coupling you should try to minimize the degree that one is bound to the other. Additionally, it's generally considered preferable for the part of the program responsible for presentation (which will typically be unique to that program) to know something about the business objects than it is for the business objects (which may get used across a wide range of applications) to know something about the presentation.

A related issue is reliance on $_SESSION inside your class. Again this has to do with separation of concerns. The $_SESSION array and its associated methods (session_start (), etc) are a data storage mechanism. Data storage is a separate concern from the business logic, so again you should try to separate the code that handles storing the game state out from the code that implements the business logic. In the case of $_SESSION there's the additional problems of it being tightly coupled to HTTP and that the session_start () function can't be called once headers have been sent to the client. Again, ask yourself how hard would it be to adapt your code to work in a command line or to use a database instead of the PHP session to store state?

StackExchange Code Review Q#29190, answer score: 8