Nothing too special here, just a bunch of stuff.  Please tell me if you would've done something different.<?php     /**     * SQL_Helper are functions which help with... SQL... :)     *     * @package    SQL_Helper     * @author     Itai Sagi     * @version    1.0     * SQL Helper functions     * Assumption: a global PDO $db is active;     */    class SQL_Helper{        // this function escapes and trims a string for insertion to SQL Database        static public function escapeSQLStatement($string){            return mysql_real_escape_string(trim($string));        }        // this function takes a query and returns a result_array if query returned results, false otherwise.        static public function result_array($query){            global $db;            $result = array();            foreach ($db->query($query) as $row){                $result[] = $row;            }            if (!empty($result)){                return $result;            }            return false;        }    }    /**     * This is the description for the class below.     *     * @package    User     * @author     Itai Sagi     * @version    1.0     * Basic User class     *      */    class User{        // class constructor which accepts a user_id as an optional parameter,         // so you won't have to use the setUser() method.        function __construct($user_id = null){            $this -> userID = $user_id;        }        /**         * function setUser($id);         * This function sets the user_id for the class         * Parameters: int $user_id, assuming a valid $user_id;          * return values: the class itself.         **/        function setUser($user_id){            $this -> userID = $user_id;            return $this;        }    }    /**     * @package User     * @subpackage Contacts     * @author Itai Sagi     * @version 1.0     * The following class handles Contacts of a user.     * Usage example:     * searching user's contacts:     * $contacts = new Contacts();     * $foundContacts = $contacts -> setUser(76) -> searchContactsByName('John');     *       * getting contacts whose birthdays are up to 3 days from now:     * please note that here, the user is still set to '76'.     * $upcomingBirthdays = $contacts -> upcomingBirthdays(3);     *      **/    class Contacts extends User{        function __construct($user_id = null){            parent::__construct($user_id);        }        /**         * function: searchContactsByName($string);         * This function searches the user's contacts for users whose name contain $string;         * parameters: String $string - the name to search for         * return values: an array containing the user's contacts which were found or FALSE if none were found or $id wasn't set         **/        function searchContactByName($string){            if ($this -> userID){                $string = SQL_Helper::escapeSQLStatement($string);                $query = SELECT lname, fname, email FROM contacts c                JOIN users u ON u.id = c.contact_id                WHERE c.id = '$this->userID' AND (lname LIKE '%$string%' OR fname LIKE '%$string%');                return $SQL_Helper::result_array($query);            }            return false;        }        /**         * function: upcomingBirthdays($daysInterval);         * This function searches the user's contacts for users whose birthday is in the upcoming $daysInterval;         * parameters: int $daysInterval - the amount of days to get the birthdays         * return values: an array containing the user's contacts which were found or FALSE if none were found or $userID wasn't set         **/        function upcomingBirthdays($daysInterval){            if ($this -> userID){                $query = SELECT lname, fname, email, birthday FROM contacts c                JOIN users u ON u.id = c.contact_id                 WHERE c.id = $this->userID AND FLOOR(                ( UNIX_TIMESTAMP(CONCAT(                        YEAR(CURDATE()) + (DATE_FORMAT(p.bday, '%m-%d') < DATE_FORMAT(CURDATE(), '%m-%d')),                         DATE_FORMAT(p.day, '-%m-%d'))) - UNIX_TIMESTAMP(CURDATE())) / 86400) < $daysInterval;                return $SQL_Helper::result_array($query);            }            return false;        }    }    /**     * @package User     * @subpackage Messages     * @author Itai Sagi     * @version 1.0     * The following class handles the messages of a user     * Usage examples:     *      * send a new message:     * $message = new Messages();     * if ($message -> setUser(65) -> sendMessage(Hello, how are you?, 12)){     *  echo Message sent successfuly;     * }     * else{     *  echo Message delivery failed;     * }     *     * getting an a list of all the messages:     * $messages = new Messages();     * $messageArr = $messages -> setUser(65) -> getMessages();     * foreach ($messageArr as $m){     *  // do something.     * }     *     **/    class Messages extends User{        function __construct($user_id = null){            parent::__construct($user_id);        }        /**         * function: getMessages();         * This function returns an array of all the messages a user recieved         * return values: an array containing the messages or FALSE on failure / no messages.         **/        function getMessages(){            if ($this -> userID){                $query = SELECT * FROM messages m                JOIN users u ON u.id = m.from_id                WHERE m.user_id = $this->userID ORDER BY send_time ASC;                return SQL_Helper::result_array($query);            }            return false;        }        /**         * function: sendMessage($message, $recipientID);         * This function sends a message from $this->userID to $recipientID         * parameters: $message - the message content, $recipientID - the target user id who should get the message         * return values: the message_id if successful or FALSE on failure         **/        function sendMessage($message, $recipientID){            if ($this -> userID){                $message = SQL_Helper::escapeSQLStatement($message);                $query = INSERT INTO messages                 (message_text, user_id, from_id, send_time) VALUES                 ('$message', $recipientID, $this->userID, NOW());                if ($db -> query ($query)){                    return $db -> lastInsertId();                };            }            return false;        }        /**         * function: searchInMessages($string)         * This function searches all the messages of a user for the text $string         * parameters: $string - the text to look for         * return values: an array containing the messages or FALSE on failure / no messages found.         **/        function searchInMessages($string){            if ($this -> userID){                // Please note that because there's a full text index on the column, we aren't performing a full                // table scan by using (LIKE '%$text')                $string = SQL_Helper::escapeSQLStatement($string);                $query = SELECT * FROM messages m                JOIN users u ON u.id = m.from_id                WHERE m.user_id = $this->userID and m.message_text LIKE '$string%' ORDER BY send_time ASC;                 return SQL_Helper::result_array($query);            }            return false;        }    }    ?>
I have a home test for job interview
php;object oriented;interview questions
Disclaimer: I am not a php developer Other posts already mentioned good points as far as specifics about SQL and php and some problems with your inheritance structure. I am going to talk more about your craftsmanship in this post. SQL_Helper are functions which help with... SQL... :)This is like going to an interview with your shirt untucked and instead of properly shaking hands you greet them with 'yo bro'. Simply unprofessional. Won't get you hired. Anyways... onward...DocumentationYour documentation is messy. Most great developers care about how their code and documentationlooks. They treat writing them as craft. You want the look/layout to be pleasing to the eye and the contents to be pleasing to the brain. This means you need to keep things consistent looking and to the point. Capitalize all your sentences. You should stick with /** */ for method/class level documentation and use // only for one-liners inside methods. Try to avoid using 'this function' when you write about the function. For example, I would rewrite // this function escapes and trims a string for insertion to SQL Database to /** Escapes and trims a string */ or /** Prepares a string to be used in an SQL statement */ (but I do not want to get caught too much in the semantics since I do not know much php...)If you absolutely have to use @package, @author and @version attributes, those should be the last thing in your documentation. They do not help the user whatsoever with using or maintaining your class. The most important things should usually go on the top. Some places you have important information both above and below and sometimes only below. I am unsure about your duplicated method signatures in your documentation (Example: * function setUser($id);) I am positive your documenation tool extracts the signature automatically from the source itself, so this is just redundant. CodeYou do not have a userID attribute on your User class. I do not know if this is standard procedure for php but even if it runs in its current form, you should specify the attributes for your classes and those should usually be private. (Standard OOP here, hide your privates)Minimize your API. You should probably choose to go with the constructor or the setterin the User class. I personally would go with the constructor. So code can be more terse by saying $contact= new Contact(72);$contact->searchContactByName('John');You should also not allow a null user when you are constructing one, so you do not have to worry about logic like this everywhere: if ($this -> userID){            ...}return false;If your User class enforces the assumption that a userId cannot be null, all that code can be eliminated. Be more creative when choosing method names. Your class name already gives semantics to most of your methods. Fo example, in SQL_Helper::escapeSQLStatement() the 'SQL' is redundant. Why not SQL_Helper::escapeStatement()? 