3
\$\begingroup\$

I've created this code to send a response to Flash which can be handled with AS2/AS3:

<?php
    /**
     * Basic server-side API controls for Flash-PHP communication
     * @author Marty Wallace
     * @version 1.0.2
     */
    class API
    {
        // MySQL
        private $con;
        private $sql_hostname = '';
        private $sql_username = '';
        private $sql_password = '';
        private $sql_database = '';

        // Properties
        public $response;

        /**
         * Constructor
         */
        public function __construct()
        {
            // Attempt connection
            $this->con = mysql_connect(
                $this->sql_hostname,
                $this->sql_username,
                $this->sql_password
            );

            // Connection could not be established
            if(!$this->con)
            {
                $this->response = array(
                    "status" => "no_connection",
                    "message" => "Could not connect to MySQL, try again later."
                );

                $this->respond();
            }

            // Select database
            mysql_select_db($this->sql_database);
        }

        /**
         * Send response back to Flash
         */
        public function respond()
        {
            $ar = array();
            foreach($this->response as $key => $value)
                array_push($ar, $key . "=" . $value);

            die(implode("&", $ar));
        }
    }
?>

I'm not that great with PHP and the respond() function seems like it could be written a little better.. Any suggestions?

\$\endgroup\$
3
\$\begingroup\$

I have used PHP for a few years now, and here are my suggestions:

array_push() can be used to add multiple values into an array at once, but in your case, your pushing only one value in at a time which means you can use:

$ar[] = $key . "=" . $value

This should also speed up your code a little( What's better to use in PHP $array[] = $value or array_push($array, $value)?).

die() is normally used when there is an error. It spits out a message to the user. If you are using this code to just connect to a MySQL database then change die(implode("&", $ar)); to echo(implode("&", $ar));

But honestly, you should be able to take out the respond() function if you are only using it to send an array to flash.

        // Connection could not be established
        if(!$this->con)
        {
            $this->response = array(
                "status" => "no_connection",
                "message" => "Could not connect to MySQL, try again later."
            );

             echo(implode("&", $this->response));
        }
\$\endgroup\$
0
\$\begingroup\$

Firstly, you don't appear to be using the database, however for my answer I will assume that you use it elsewhere in the class.

With OOP it is good to use Dependency Injection. Dependency injection helps to ensure that the objects you create are only responsible for what they need to be.

Note how your API object is currently responsible for creating a connection to a database. Lets look at the Pros and Cons for this:

  • Pros - None that I can see (sorry), although you could say that everything is done in the one place for the API object (I see this as a disadvantage).
  • Cons - Tightly coupled to mysql database, API object has responsibility for database actions, using mysql_* which is now softly deprecated, code duplication for database actions, password connection details are spread throughout the classes rather than being in one place.

Here is how your code would look using Dependency Injection:

class API
{
    protected $db;

    /**
     * Constructor
     */
    public function __construct($db)
    {
        $this->db = $db;
    }

    /**
     * Send response back to Flash
     */
    public function respond()
    {
        // This is rewritten as Chillie suggested.
        $ar = array();
        foreach($this->response as $key => $value)
            $ar[] = $key . "=" . $value;

        echo implode("&", $ar);
    }
}

I would handle database connection errors at the usage level:

try
{
    $FlashResponder = new API(new PDO($dsn, $user, $password));
}
catch (Exception $ex)
{
    echo "status=no_connection&message=Could not connect to database, try again later.";

}

Dependency Injection now allows any database to be used by the API object. When the database connection details change or a new database needs to be connected to the values are no longer hardcoded into the API class making maintenance easier. The code for the API class is also simpler as the database code was making it harder to read and understand.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.