Object-oriented command processor

Question

I really need a review on the structure, and how things are done. I want to improve.

Command.class:

<?php

    /**
     * Command
     * Processing commands to the server, acting, receiving response.
     *
     * @author Jony <[email protected]>
     */

    class Command extends PrepareCommand
    {
        /**
         * Client
         * The command processor
         * @object Client
         */

        private $client;

        /**
         * Annonymous constructor
         * Constructs the application.
         *
         * @param message   ; Object Client
         */

        public function __construct(Client $message)
        {
            $this->client = $message;

            if ($this->isUsingCommand())
            {
                $this->_prepare();
            }
        }

        /**
         * Protected _prepare
         * Preparing the command line.
         *
         * @return void
         */

        protected function _prepare() 
        {
            $this->_commander = $this->client;
            $this->_process();
            $this->processCommand();
        }

        /**
         * processCommand
         * Processing the given commmand, finds out
         * what command was used, and it's data.
         *
         * @var _commandLine    ; The used command name.
         * @var _commandActions ; The data that comes after the command
         * @return void
         */

        private function processCommand()
        {
            switch ($this->_commandLine[1])
            {
                case "ban":
                    echo "LOl you tried to ban " . $this->fullString();
                    break;

                case "yell":
                    echo $this->fullString();
                    break;
            }
        }

        /**
         * isUsingCommand
         * Is the Client using a command at all?
         * Checks if line starts with the command sign " / ".
         *
         * @return boolean
         */

        public function isUsingCommand() 
        {
            return (substr($this->client->getMessage(), 0, 1) === "/") ? true : false;
        }

        /**
         * fullString
         * Gets all of the array elements of _commandActions and
         * converts it to one big String
         *
         * @var message         ; The String message.
         * @var _commandActions ; The data that comes after the command.
         * @return String
         */

        private function fullString()
        {
            $message = "";
            for ($i = 1; $i < count($this->_commandActions); $i++)
            {
                $message .= " " . $this->_commandActions[$i];
            }
            return $message;
        }


    }

?>

PrepareCommand.class:

<?php

    abstract class PrepareCommand 
    {

        /**
         * Properties
         */

        private $command;
        protected $_commander;
        protected $_commandActions;
        protected $_commandLine;

        /**
         * Processing the command, and calling parse
         * to explore the commandline.
         *
         * @return void
         */

        protected function _process()
        {
            $this->command = $this->_commander->getMessage();
            $this->parse();
        }

        /**
         * Parsing the command ; Exploring it ; 
         * Getting the first command and actions.
         *
         * @var _commandActions     ; Holding all of the words, between the entered spaces.
         * @var _commandLine        ; Getting the entered command name after the " / " sign.
         * @return void
         */

        private function parse() 
        {
            $this->_commandActions = explode(" ", $this->command);
            $this->_commandLine = explode("/", $this->_commandActions[0]);
        }   

        /**
         * Abstract prapre
         *
         * Activating the class, and preparing the command
         * through the class.
         *
         * @return void
         */

        protected abstract function _prepare();

    }

?>

usage:

new Command(new Client($_POST['process'], date('time'), "guest"));

Answer 1

1. Stop using underscore to indicate private/protected parts of a class.
2. Do not put logic in the constructors
3. It is pointless to have ... ? true : false; in isUsingCommand().
4. Use implode() in fullString()

Answer 2

Such comments are obvious and help very little to read the code:

    /**
     * Annonymous constructor
     * Constructs the application.

Uncle Bob in his seminal Book "Clean Code" recommends to replace comments (that are often imprecise and tend to lie once code changes) as much as possible by expressive names of your methods.

For instance call your method _prepareCommandLine instead of just _prepare, then you don't need to put in the comments. Moreover, should you decide to change this method, you much more likely to change its name to reflect the new function, but can easily forget to change the comment.

---

This is well-known anti-pattern:

for ($i = 1; $i < count($this->_commandActions); $i++)

This way the count function executes in every single loop. You can avoid this:

for ($i = 1, $max = count($this->_commandActions); $i < $max; $i++)

However, if your loop only concatenates array, using implode is even better as suggested by tereško.

---

Why underscore for protected but none for private?

    private $command;
    protected $_commander;

That would make more sense:

    private $__command;
    protected $_commander;

---

Again, the name says little about what's going on:

private function parse() 

---

These comments are again obvious and worse, have misprints:

    /**
     * Abstract prapre
     *

This type of line is way too concrete for an abstract class:

        $this->_commandLine = explode("/", $this->_commandActions[0]);

You may want to revise your abstractions :)