1
\$\begingroup\$

I've been developing sites using Dreamweaver for the last 15 years, I do a lot of code editing manually so I have quite a good knowledge of the PHP language.

I'm diving into object orientated programming and getting to grips with classes objects and the like.

I hope to eventually port the game to a framework such as symfony, I feel at this time there's a lot more to learn before I start diving into frameworks, and would just like to concentrate on getting to grips with OOP, without having to worry about templating frameworks, ORM etc.

The game itself is quite simple, its based on a party game called Mafia http://en.wikipedia.org/wiki/Mafia_(party_game)

There's a certain amount of players required, once enough players have joined, the game starts and people vote who they want to eliminate.

There are 2 periods of voting during the day and a night round where certain players can vote at night.

I've created the initial user login (for which I am using Facebook's PHP SDK).

I just wanted to get some advice on the best way to go before I get to far ahead of myself.

I think I am probably adding to much functionality to my game class

What's the best way to break it down?

Most of the data for the game is stored in a MySQL database, where I am using PDO (moving away from the horrendously insecure way that Dreamweaver implements).

Some of the questions I Have along the way are:

Do I create separate classes for the data in the database.

For example for my game table just have a class that fetches a list of games that i can then iterate through and display them as a list on my page. within this class I can create methods to join a game, create a game, leave a game. each method would return the game object $this.

Have a class that contains a list of users in the current game, then within that class have methods for processing votes or do i have a separate class called gameFunctions that receives the current game, and list of users in that game with methods that perform the game functions such as voting.

I have already started working on it, and what I have so far is below, any tips or suggestions on how to separate the code so its more manageable would be great along with any other advice or resources that would be helpful to my learning process.

User Class:

<?php

require_once('facebook/facebook.php'); // require facebook sdk
require_once('config.class.php'); // require config class
require_once('dbconn.class.php'); //require database connector

class user{
    public $_userData; // holds information of our user
    private $conn; // our database connection
    public $facebook; // facebook sdk
    public $fbuser; // the facebook user
    //Constructor
    public function __construct(){
        $this->conn = DBconn::getInstance(); //get DB instance
        $this->facebook = new Facebook(Config::read('fb.facebook'));
        $this->_userData = new stdClass();
    }

    /**
     * CheckAccess - checks to see if the user is logged in to the session
     * @return bool:
     * true: user logged in
     * false: user not logged in
     */
    public function checkAccess(){
        if(session_id() == '') {
            session_start(); // Start session if we don't already have one
        }
        if (isset($_SESSION['loggedin']) && $_SESSION['loggedin'] == true) {
            return true;
        }   else{
            return false;
        }
    }

    /**
     * profileExists - Checks to see if a user profile exists
     * @return bool: true/false
     */
    private function profileExists(){
        try{
            $pdoQuery = ("SELECT count(*) FROM userprofile_upl WHERE uid_upl = :uid");
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->bindParam(':uid', $this->_userData->uid);
            $stmt->execute();
            if($stmt->fetchColumn()){
                return true;
            }else{
                return false;
            }
        }catch(PDOException $e){
            echo('There was an error while connecting to your profile<br>');
            return false;
        }
    }

    /**
     * getProfile - querys the database and stores the users profile in $_userData
     * @return bool
     */
    public function getProfile(){
        try{
            $pdoQuery = ("SELECT
                uid_upl as'uid',
                firstname_upl as 'firstname',
                lastname_upl as 'lastname',
                info_upl as 'info'
                FROM userprofile_upl
                WHERE uid_upl = :uid"
            );
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->bindParam(':uid', $this->_userData->uid);
            $stmt->execute();
            $row = $stmt->fetch(PDO::FETCH_OBJ);
            $this->_userData->firstname = $row->firstname;
            $this->_userData->lastname = $row->lastname;
            $this->_userData->info = $row->info;
            return (true);
        }catch(PDOException $e){
            echo('There was an error while connecting to your profile<br>');
            return false;
        }
    }

    /**
     * createProfile - Creates a user profile
     * @param $user object: The Users Data
     * @return bool: true/false
     */
    private function createProfile(){
            try{
                //we need to add the facebook user to the profiles table
                $pdoQuery = ('INSERT INTO userprofile_upl (firstname_upl, lastname_upl, uid_upl) VALUE (:firstname, :lastname, :uid)');
                $pdoData = array(
                    'firstname' => $this->_userData->firstname,
                    'lastname' => $this->_userData->lastname,
                    'uid' => $this->_userData->uid
                );
                $stmt = $this->conn->dbh->prepare($pdoQuery);
                $stmt->execute($pdoData);
                return true;
            }catch(PDOException $e){
                echo('There was an error while creating the profile<br>');
                return false;
            }
    }

    /**
     * goDashboard - Redirects a logged in user to the dashboard if they are logged in
     */
    public function goDashboard(){
        if($this->checkAccess()){
            header('Location: '.Config::read('url.dashboard'));
        }else{
            return false;
        }
    }

    /**
     * generates a facebook log-out url
     * @return String: facebook logout url
     */
    public function getLogoutUrl(){
        //set user logout_url
        $params = array( 'next' => config::read('url.logout'));
        $this->_userData->logout_url = $this->facebook->getLogoutUrl($params);
        return $this->_userData->logout_url;
    }

    /**
     * loginFacebook - Logs a user into the site with facebook
     * Checks to see if user has an account and if not make one.
     * Checks to see if user has a profile and if not make one.
     * Checks to see if user has facebook credentials on this site
     * @return mixed
     */
    public function loginFacebook(){
        $fbuser = $this->facebook->getUser();//get facebook user
        if($fbuser){// if we have a user returned we have user who's authenticated and logged in with facebook
            try { // Proceed knowing we have a user.
                $me = $this->facebook->api('/me'); // the facebook user
                //generate a log-out url

                //set user logout_url
                $this->_userData->logout_url = $this->facebook->getLogoutUrl($params);
                //set the users details
                $this->_userData->uid = $this->facebook->getUser();
                $this->_userData->firstname = $me['first_name'];
                $this->_userData->lastname = $me['last_name'];
                $this->_userData->email = $me['email'];
                $this->_userData->provider = 'facebook';
                echo(" facebook account exists: ".$this->fbAccountExists());
                if(!$this->fbAccountExists($this->_userData)){
                    echo("no fb account exists");
                    $this->fbCreateAccount($this->_userData);
                };
                echo("profile exists: ".$this->profileExists());
                if(!$this->profileExists($this->_userData)){
                    echo("no profile exists: ");
                    $this->createProfile($this->_userData);
                }
                $this->setSession();
            }
            catch (FacebookApiException $e){ //if theres an error
                $fbuser = null; //set the user to nothing
                return false();
            }
        }
        if(!$fbuser){
            //get a login url, returns to this page so we can process the login($fblogin_url)
            $loginUrl = $this->facebook->getLoginUrl(array('redirect_uri'=>Config::read('url.fblogin'), false));
            header('Location: '.$loginUrl);//redirect to the login page
        }
    }

    /**
     * fbAccountExists - checks to see if a facebook account exists
     * @return bool: true/false
     */
    private function fbAccountExists(){
        try{
            $pdoQuery = ("SELECT count(*) FROM oauth_auth WHERE uid_auth = :uid and provider_auth = 'facebook'");
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->bindParam(':uid', $this->_userData->uid);
            $stmt->execute();
            if($stmt->fetchColumn()){
                return true;
            }else{
                return false;
            }
        }catch(PDOException $e){
            echo('There was an error while connecting to your profile<br>');
            return false;
        }
    }

    /**
     * fbCreateAccount - creates a facebook account
     * @return bool: true/false
     */
    private function fbCreateAccount(){
        try{
            //we need to add the facebook user to the auth table
            $pdoQuery = ("INSERT INTO oauth_auth(email_auth, uid_auth, provider_auth) VALUE (:email, :uid, :provider)");
            $pdoData = array('email' => $this->_userData->email, 'uid' => $this->_userData->uid, 'provider' => $this->_userData->provider);
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->execute($pdoData);
            return true;
        }catch(PDOException $e){
            echo('There was an error while connecting to your profile<br>');
            return false;
        }
    }

    /**
     * setSession - Stores session variables to imitate user login.
     */
    function setSession(){
        if(session_id() == '') {session_start();}// Start session if we dont already have one
        $_SESSION['loggedin'] = true;
        $this->_userData->loggedin = true;
        $_SESSION['uid'] = $this->_userData->uid;
        $_SESSION['provider'] = $this->_userData->provider;
        header('Location: '.Config::read('url.dashboard'));
    }

    /**
    * getSession - saves session vars to our user
    * @return bool: true - session retrieved
    * @redirect if no session found redirects to login page
    */
    public function getSession(){
        if(session_id() == '') {session_start();}// Start session if we dont already have one
        if (!isset($_SESSION['loggedin']) && $_SESSION['loggedin'] != true) {//Check to see if user is logged in
            header("location: ".Config::read('url.baseurl')); // User is logged in Redirect to homepage
        }else{
            //save session to this user
            $this->_userData->loggedin = $_SESSION['loggedin'];
            $this->_userData->provider = $_SESSION['provider'];
            $this->_userData->uid = $_SESSION['uid'];
            return true;
        }
    }




}

Game Class:

<?php
/**
 * Created by Base5 Designs.
 * and Dizzy Developments
 * User: Dizzy High
 * Date: 14/04/14
 * Time: 12:08
 */

require_once('config.class.php'); // require config class
require_once('dbconn.class.php'); //require database connector

class game {
    public $conn; // database handler
    public $gameID; // id of game
    public $name; //name of game
    public $host; // uid of game host (the game creator)
    public $playerCount; // number of players in game
    public $roundTime; //round time of game in seconds
    public $visibility; // public (1) or private game  (0)
    public $alias; // whether game players use an alias name or not (1 yes 2 no)
    public $status; //status of game (waiting, active, complete)
    public $currentRound; // current round of game (0 not started, odd day, even night)
    public $startTime; // datetime - start time of game
    public $createdTime; // datetime date game created

    function __construct(){
        $this->conn = DBconn::getInstance(); //get DB instance
    }

    /**
     * assignSettings - assigns settings
     * @param $name - property name
     * @param $value - property value
     */
    function assignSetting($name, $value){
        $whitelist = array('id', 'name', 'host', 'playerCount', 'roundTime', 'visibility', 'alias', 'status', 'currentRound', 'startTime', 'createdTime');

        if (in_array($name, $whitelist)) {
            $property = $name;
            $this->$property = $value;
        }
    }

    /**
     * makeGame- creates a new game and adds it to the database with the defined settings
     * @param $settings
     * @return bool
     */
    function makeGame($settings){
        //var_dump($settings);
        if (is_array($settings)) {
            foreach($settings as $k=>$v)  {
                $this->assignSetting($k, $v);
            }
        } else {
            die('Config Error!');
        }
        try{
            //add the game to the games table
            $pdoQuery = ('INSERT INTO games_gam (name_gam, host_gam, playercount_gam, roundtime_gam, visibility_gam, alias_gam) VALUE (:gameName, :gameHost, :playerCount, :roundTime, :visibility, :alias)');
            $pdoData = array(
                'gameName' => $this->name,
                'gameHost' => $this->host,
                'playerCount' => $this->playerCount,
                'roundTime' => $this->roundTime,
                'visibility' => $this->visibility,
                'alias' => $this->alias
            );
            var_dump($pdoData);
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->execute($pdoData);
            $this->gameID = $this->conn->dbh->lastInsertId();
            //add creator to game
            $this->addPlayer($this->host);
            return $this;
        }catch(PDOException $e){
            echo('There was an error while creating the game<br>');
            return false;
        }
    }

    /**
     * getGame - retrieves a game from the database
     * @param $gameID INT -  the id of the game
     * @return $this
     */
    function getGame($gameID){
        try{
            $pdoQuery = ("SELECT
                id_gam as 'id',
                name_gam as 'name',
                host_gam as 'host',
                playercount_gam as 'playerCount',
                roundtime_gam as 'roundTime',
                visibility_gam as 'visibility',
                status_gam as 'status',
                alias_gam as 'alias',
                starttime_gam as 'startTime',
                createdtime_gam as 'createdTime'
                FROM games_gam
                WHERE id_gam = :id"
            );
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->bindParam(':id', $gameID);
            $stmt->execute();
            $row = $stmt->fetch(PDO::FETCH_OBJ);
            $this->gameID = $row->id;
            $this->name = $row->name;
            $this->host = $row->host;
            $this->playerCount = $row->playerCount;
            $this->roundTime = $row->roundTime;
            $this->visibility = $row->visibility;
            $this->status = $row->status;
            $this->alias = $row->alias;
            $this->startTime = $row->startTime;
            $this->createdTime = $row->createdTime;
            return ($this);
        }catch(PDOException $e){
            echo('There was an error while connecting to game: '.$gameID.'<br>');
            return false;
        }
    }

    /**
     * addPlayer - adds player to a game
     * @param $uid - uid of player
     * @return bool
     */
    function addPlayer($uid){
        if(!$this->playerInGame($uid)){
            try{
                $pdoQuery = ('INSERT INTO gameplayers_gpl (idgam_gpl, uid_gpl) VALUE (:idgam, :uid)');
                $pdoData = array(
                    ':idgam' => $this->gameID,
                    ':uid' => $uid
                );
                $stmt = $this->conn->dbh->prepare($pdoQuery);
                $stmt->execute($pdoData);
                return true;
            }catch(PDOException $e){
                echo('There was an error while creating the profile<br>');
                return false;
            }
        }
    }

    /**
     * leaveGame - Removes a player from a game
     * @param $gameID - the id of the game
     * @param $uid - the uid of the player
     * @return $this
     */
    function leaveGame($uid){
        if($this->playerInGame($uid)){
            try{
                $pdoQuery = ('DELETE FROM gameplayers_gpl WHERE uid_gpl = :uid AND idgam_gpl = id');
                $pdoData = array(
                    ':uid' => $uid,
                    ':id'=> $this->$gameID
                );
                $stmt = $this->conn->dbh->prepare($pdoQuery);
                $stmt->execute($pdoData);
                return $this;
            }catch(PDOException $e){
                echo('There was an error while removing the player from the game<br>');
                return false;
            }
        }
    }

    /**
     * gameFull - checks to see if a game has all player slots filled
     * @return bool true: game is full
     * @return bool false: has slots
     */
    function gameFull(){
        try{
            $pdoQuery = ("SELECT count(*) FROM gameplayers_gpl WHERE idgam_gpl = :gameID");
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $stmt->bindParam(':gameID', $this->gameID);
            $stmt->execute();
            $rowCount = (int) $stmt->fetchColumn();
            if($rowCount < $this->playerCount){
                return false;
            }else{
                return true;
            }
        }catch(PDOException $e){
            echo('There was an error while checking if the game was full<br>');
            return false;
        }
    }

    /**
     * playerInGame - checks to see if a player in the game
     * @param $uid: id of user
     * @return bool
     */
    function playerInGame($uid){
        try{
            $pdoQuery = ("SELECT count(*) FROM gameplayers_gpl WHERE idgam_gpl = :gameID AND uid_gpl = :uid");
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $pdoData = array(
                ':uid' => $uid,
                ':gameID'=> $this->gameID
            );
            $stmt->execute($pdoData);
            $rowCount = (int) $stmt->fetchColumn();
            if($rowCount > 0){
                return true;
            }else{
                return false;
            }
        }catch(PDOException $e){
            echo('There was an error while checking if the user was in the game<br>');
            return false;
        }
    }

    function getGamesList($roundtime){
        try{
            $pdoQuery = ("SELECT
                id_gam as 'id',
                name_gam as 'name',
                host_gam as 'host',
                playercount_gam as 'playerCount',
                roundtime_gam as 'roundTime',
                visibility_gam as 'visibility',
                status_gam as 'status',
                alias_gam as 'alias',
                starttime_gam as 'startTime',
                createdtime_gam as 'createdTime'
                FROM games_gam
                WHERE visibility_gam = :visibility AND roundtime_gam = :roundtime AND status_gam = :gamestatus"
            );
            $stmt = $this->conn->dbh->prepare($pdoQuery);
            $pdoData = array(
                'visibility' => 0,
                'roundtime' => $roundtime,
                'gamestatus' => 'waiting',
            );
            $stmt->execute($pdoData);
            $obj = $stmt->fetchAll();
            return ($obj);
        }catch(PDOException $e){
            echo('There was an error while connecting to game: '.$gameID.'<br>');
            return false;
        }

    }
    /**
    * Convert number of seconds into years, days, hours, minutes and seconds
    * and return an string containing those values
    *
    * @param integer $seconds Number of seconds to parse
    * @return string
     */
    function secondsToTime($seconds){
        $y = floor($seconds / (86400*365.25));
        $d = floor(($seconds - ($y*(86400*365.25))) / 86400);
        $h = gmdate('H', $seconds);
        $m = gmdate('i', $seconds);
        $s = gmdate('s', $seconds);

        $string = '';

        if($y > 0){
            $yw = $y > 1 ? ' years ' : ' year ';
            $string .= $y . $yw;
        }
        if($d > 0){
            $dw = $d > 1 ? ' days ' : ' day ';
            $string .= $d . $dw;
        }
        if($h > 0){
            $hw = $h > 1 ? ' hours ' : ' hour ';
            $string .= $h . $hw;
        }
        if($m > 0){
            $mw = $m > 1 ? ' minutes ' : ' minute ';
            $string .= $m . $mw;
        }
        if($s > 0){
            $sw = $s > 1 ? ' seconds ' : ' second ';
            $string .= $s . $sw;
        }

        return preg_replace('/\s+/',' ',$string);
    }

    function getHost($gameID){

    }
    function initGame(){

    }

    function processVotes(){

    }

    function setPlayerRole($uid, $role){


    }

    function getPlayerRole(){

    }

    function getPlayerHealth(){

    }

    function getGameDay(){

    }

    function getGameVotes(){

    }
\$\endgroup\$
  • 2
    \$\begingroup\$ I think the rule of "class names should start with a capitalized word" apply in php too User and Game \$\endgroup\$ – Marco Acierno Apr 18 '14 at 13:38
3
\$\begingroup\$

Formatting

@MarcoAcierno's comment is correct, and you also have a few other formatting errors.

We can start with capitalization! I'll quote a couple things here that could be considered a standardized practice:

If a class name is comprised of more than one word, the first letter of each new word must be capitalized.

So, user -> User and game -> Game.

Variables that are declared with the "private" or "protected" modifier, the first character of the variable name must be a single underscore. This is the only acceptable application of an underscore in a variable name. Member variables declared "public" should never start with an underscore.

In relation to this, there are a couple variables that need new names. In both files, you control your PDO statements with the variable $stmt. This is very ambiguous and a simple, yet tremendously vague name. To make it easier to understand, give the variables some meaning.

As with classes, the brace should always be written on the line underneath the function name.

Just nit-picking here. It increases readability though.

When a string is literal (contains no variable substitutions), the apostrophe or "single quote" should always be used to demarcate the string

This one you had followed for the most part, except for your queries, all of which are on double quotes.

The opening brace is written on the same line as the conditional statement. The closing brace is always written on its own line.

You've condensed a couple if statements into one liners, which only makes it more difficult to follow.

I've also noticed you've added some documentation. Good start, but it needs some fine tuning to actually be efficient. I think a Google for PHPDoc will give you a good insight at to what comments should look like. Here's a good section on that too.

Files/Classes

I'd agree with "I think I am probably adding to much functionality to my game class."

To me, your Game class is handling database queries, game logic, and presentation.

To rid of the database queries, I'd say you could build a database class, something that frameworks usually come with. You want to decouple the game and the data as much as possible.

So hopefully from your Game class, you'd only be calling something such as:

function gameFull()
{
    $playersInGameQuery = 'SELECT count(*) FROM gameplayers_gpl WHERE idgam_gpl = :gameID';
    if (!$playersInGame = $someDatabaseClass->query($playersInGameQuery, array(':gameID' => $this->gameID))->fetchColumn()) {
        throw new DataBaseException('Checking for player amount failed.');
    }

    if ($playersInGame < $this->playerCount) {
        return false;
    }
    return true;

}

Which brings us to a new subject. Your Game class is also controlling the presentation. It's imperative that classes handle SOLID rules. I'd suggest taking your PDOExceptions and placing them in your database class, and having that class throw more specific exceptions. Then have your View file handle those exceptions. It's not always necessary that you create exceptions from scratch, but here's PHP's page on custom exceptions. Or you may be able to work with a preset exception.

Remember, no echoing in a business logic or data access layer.

And then if we have removed data and presentation, then the Game class is left with game logic! Now it accepts user actions and determines what to do with those actions.

Coding

Let's simplify some code now! Starting off with checkAccess().

if(session_id() == '') could be if (empty(session_id())). And then

if (isset($_SESSION['loggedin']) && $_SESSION['loggedin'] == true) {
    return true;
} else{
    return false;
}

can be simplified to:

return isset($_SESSION['loggedin']) && $_SESSION['loggedin'] === true;

Notice how I made == into an exact type match too. If not, loggedin could be 5 and still pass!

Same type of makeover can happen in profileExists():

return $stmt->fetchColumn() > 0;

Although, if you're to add a Database layer, this may not be required.

return (true); is overkill. No parentheses required.

In goDashboard() and other functions, you could simply write:

if ($this->checkAccess()) {
    header('Location: ' . Config::read('url.dashboard'));
}
return false;

I'm not sure why you're suppressing this error:

catch (FacebookApiException $e){ //if theres an error
    $fbuser = null; //set the user to nothing
    return false();
}

You don't need the parentheses after false.

Your last function in your Game class, secondsToTime(), could use some factoring!

All you really need is the DateTime class and you can create something such as:

$zeroTime = new DateTime('@0');
$newInputTime = new DateTime("@$seconds");
$humanTime = $zeroTime->diff($newInputTime)->format('%y years, %a days, %h hours, %i minutes and %s seconds');

return preg_replace('/(0 \w+, )/', '', preg_replace('/(1 \w+)s/', '$1', $humanTime));

SQL

Your queries look pretty good, so I'm just going to nit-pick here.

From profileExists(), you can alter your query to be a little bit more efficient:

SELECT count(1) FROM `userprofile_upl` WHERE `uid_upl` = :uid;

All I've done is take away the * selector, so now it's not counting all columns, yet it returns the same thing. I recommend using backticks, even if you don't have any special characters. Just a good habit.

In this query:

SELECT
    `firstname_upl` as `firstname`,
    `lastname_upl` as `lastname`,
    `info_upl` as `info`
    FROM `userprofile_upl`
    WHERE `uid_upl` = :uid;

I took out the the uid select, you don't use it anywhere else in the function.

In leaveGame(), you have:

DELETE FROM gameplayers_gpl WHERE uid_gpl = :uid AND idgam_gpl = id

And I'm assuming it's a typo where you forgot the colon in front of id. You may want to have it or else face an issue debugging.

\$\endgroup\$
  • \$\begingroup\$ Thankyou very much for this, i will go through my code and make the changes you suggest, might take me a day or two, i will update the question with the modified code when its done. I really appreciate your time looking at this i have found your comments most helpful. \$\endgroup\$ – Dizzy Bryan High Apr 19 '14 at 16:24
  • \$\begingroup\$ Of course, when you update, don't overwrite your original code. That way users may see changes made. \$\endgroup\$ – Alex L Apr 19 '14 at 16:33
  • \$\begingroup\$ I have a question about: When a string is literal (contains no variable substitutions), the apostrophe or "single quote" should always be used to demarcate the string. In my queries i need to adjust the single quotes ' within the query to double quotes " so it becomes 'SELECT mycolumn_col AS "newname" FROM mytable' otherwise this would break the query? \$\endgroup\$ – Dizzy Bryan High Apr 19 '14 at 17:34
  • \$\begingroup\$ @DizzyBryanHigh when adding an alias, use back-ticks. Then you'll have back-ticks inside single quotes which works fine. \$\endgroup\$ – Alex L Apr 19 '14 at 17:41
  • \$\begingroup\$ Hmm i read on the PHPdoc link you gave When a literal string itself contains apostrophes, it is permitted to demarcate the string with quotation marks or "double quotes". This is especially useful for SQL statements: but i shall use backticks ``` as you suggest, it seems to keep things more alike... \$\endgroup\$ – Dizzy Bryan High Apr 19 '14 at 17:44
1
\$\begingroup\$

Once again thanks for your time, ive gone through my code and made some changes as per your recommendations, i might have missed some things out, but i think its a lot better than it was.

The PHPDoc is great my ide (PHPStorm 6)now auto-suggests correctly where it didnt before, so already my coding is becoming more efficient!!!

I have not implented your seconds to time function, as some game will be 24hrs long per turn, and i belive that php datetime can only handle seconds that are less than a day. if i'm wrong on this i will implement your suggestion as its much cleaner than what i have written!!

User Class:

<?php

require_once('facebook/facebook.php'); // require facebook sdk
require_once('config.class.php'); // require config class
require_once('dbconn.class.php'); //require database connector

class User
{
    /**
     * @var $userData stdClass
     * @var $_conn customSingleton
     * @var $facebook Facebook - facebook SDK
     * @var $user - Facebook user
     */
    public  $userData;
    private $_conn; // our database connection
    public  $facebook; // facebook sdk
    public  $fbuser; // the facebook user
    //Constructor

    public function __construct()
    {
        $this->_conn = DBconn::getInstance(); //get DB instance
        $this->facebook = new Facebook(Config::read('fb.facebook'));
        $this->_userData = new stdClass();
    }

    /** checkAccess
     * checks to see if user has logged in and has access to the site
     * @return bool
     */
    public function checkAccess()
    {
        //cant use if(empty(session_id)) as empty and isset both require a variable to be passed
        if(!session_id()){
            session_start(); // Start session if we don't already have one
            return (isset($_SESSION['loggedin']) && $_SESSION['loggedin'] === true);
        }
    }

    /**
     * profileExists - Checks to see if a user profile exists
     * internal @var $PDOQuery
     * @var $PDOStatement PDOStatement
     * @throws Exception - details of PDOException
     * @return bool
     */
    private function profileExists()
    {
        try{
            /**  */
            $PDOQuery = ('SELECT count(1) FROM userprofile_upl WHERE uid_upl = :uid');
            $PDOStatement = $this->_conn->dbh->prepare($PDOQuery);
            $PDOStatement->bindParam(':uid', $this->_userData->uid);
            $PDOStatement->execute();
            return $PDOStatement->fetchColumn() >0;
        }catch(PDOException $e){
            throw  new Exception('Failed to check if profile exists for '.$this->_userData->uid, 0, $e);
        }
    }

    /**
     * getProfile - querys the database and stores the users profile in $userData
     * @throws Exception
     * @internal param $pdoQuery
     * @internal @var $row User
     * @internal param PDOStatement $PDOStatement
     * @throws Exception
     * @return bool
     */
    public function getProfile()
    {
        try{
            $pdoQuery = ('SELECT
                firstname_upl as `firstname`,
                lastname_upl  as `lastname`,
                info_upl      as `info`
                FROM userprofile_upl
                WHERE uid_upl = :uid'
            );
            $PDOStatement = $this->_conn->dbh->prepare($pdoQuery);
            $PDOStatement->bindParam(':uid', $this->_userData->uid);
            $PDOStatement->execute();
            $row = $PDOStatement->fetch(PDO::FETCH_OBJ);
            $this->_userData->firstname = $row->firstname;
            $this->_userData->lastname = $row->lastname;
            $this->_userData->info = $row->info;
            return true;
        }catch(PDOException $e){
            throw new Exception('Failed to get Profile for user'.$this->_userData->uid, 0, $e);
        }
    }

    /**
     * createProfile - Creates a user profile
     * @internal param $pdoQuery
     * @internal @var $pdoData array
     * @throws Exception
     * @return bool
     */
    private function createProfile()
    {
        try{
            //we need to add the facebook user to the profiles table
            $pdoQuery = ('INSERT INTO userprofile_upl (firstname_upl, lastname_upl, uid_upl) VALUE (:firstname, :lastname, :uid)');
            $pdoData = array(
                'firstname' => $this->_userData->firstname,
                'lastname' => $this->_userData->lastname,
                'uid' => $this->_userData->uid
            );
            $pdoStmt = $this->_conn->dbh->prepare($pdoQuery);
            $pdoStmt->execute($pdoData);
            return true;
        }catch(PDOException $e){
            throw new Exception('Failed add Profile for user'.$this->_userData->uid, 0, $e);
        }
    }

    /**
     * goDashboard - Redirects a logged in user to the dashboard if they are logged in
     * @return bool
     */
    public function goDashboard()
    {
        if($this->checkAccess()){
            header('Location: '.Config::read('url.dashboard'));
        }else{
            return false;
        }
    }

    /**
     * generates a facebook log-out url
     * @return String: facebook logout url
     */
    public function getLogoutUrl()
    {
        //set user logout_url
        $params = array( 'next' => config::read('url.logout'));
        $this->_userData->logout_url = $this->facebook->getLogoutUrl($params);
        return $this->_userData->logout_url;
    }

    /**
     * loginFacebook - Logs a user into the site with facebook
     * Checks to see if user has an account and if not make one.
     * Checks to see if user has a profile and if not make one.
     * Checks to see if user has facebook credentials on this site
     *
     * @throws Exception
     * @return mixed
     */
    public function loginFacebook()
    {
        $fbuser = $this->facebook->getUser();//get facebook user
        if($fbuser){// if we have a user returned we have user who's authenticated and logged in with facebook
            try { // Proceed knowing we have a user.
                $me = $this->facebook->api('/me'); // the facebook user
                //generate a log-out url

                //set user logout_url
                $this->_userData->logout_url = $this->facebook->getLogoutUrl($params);
                //set the users details
                $this->_userData->uid = $this->facebook->getUser();
                $this->_userData->firstname = $me['first_name'];
                $this->_userData->lastname = $me['last_name'];
                $this->_userData->email = $me['email'];
                $this->_userData->provider = 'facebook';
                if(!$this->fbAccountExists($this->_userData)){
                    $this->fbCreateAccount($this->_userData);
                };
                if(!$this->profileExists($this->_userData)){
                    $this->createProfile($this->_userData);
                }
                $this->setSession();
            }
            catch (FacebookApiException $e){ //if theres an error
                throw new Exception('Failed to Login to facebook '.$this->_userData->uid, 0, $e);
            }
        }
        if(!$fbuser){
            //get a login url, returns to this page so we can process the login($fblogin_url)
            $loginUrl = $this->facebook->getLoginUrl(array('redirect_uri'=>Config::read('url.fblogin'), false));
            header('Location: '.$loginUrl);//redirect to the login page
        }
    }

    /**
     * fbAccountExists - checks to see if a facebook account exists
     * @internal @var $pdoQuery
     * @internal @var $pdoStatement STRING
     * @throws Exception
     * @return bool
     */
    private function fbAccountExists()
    {
        try{
            $pdoQuery = ('SELECT count(1) FROM oauth_auth WHERE uid_auth = :uid and provider_auth = `facebook`');
            /** */
            $pdoStatement = $this->_conn->dbh->prepare($pdoQuery);
            $pdoStatement->bindParam(':uid', $this->_userData->uid);
            $pdoStatement->execute();
            return $pdoStatement->fetchColumn() > 0;
        }catch(PDOException $e){
            throw new Exception('Failed to check if account exists for user '.$this->_userData->uid, 0, $e);
        }
    }

    /**
     * fbCreateAccount - creates a facebook account
     * @throws Exception
     * @return bool: true/false
     */
    private function fbCreateAccount()
    {
        try{
            //we need to add the facebook user to the auth table
            $pdoQuery = ("INSERT INTO oauth_auth(email_auth, uid_auth, provider_auth) VALUE (:email, :uid, :provider)");
            $pdoData = array('email' => $this->_userData->email, 'uid' => $this->_userData->uid, 'provider' => $this->_userData->provider);
            $pdoStmt = $this->_conn->dbh->prepare($pdoQuery);
            $pdoStmt->execute($pdoData);
            return true;
        }catch(PDOException $e){
            throw new Exception('There was an error creating an account for '.$this->_userData->uid, 0, $e);
        }
    }

    /**
     * setSession - Stores session variables to imitate user login.
     */
    function setSession()
    {
        if(session_id() == '') {session_start();}// Start session if we dont already have one
        $_SESSION['loggedin'] = true;
        $this->_userData->loggedin = true;
        $_SESSION['uid'] = $this->_userData->uid;
        $_SESSION['provider'] = $this->_userData->provider;
        header('Location: '.Config::read('url.dashboard'));
    }

    /**
     * getSession - saves session vars to our user
     * @return bool: true - session retrieved
     * @redirect if no session found redirects to login page
     */
    public function getSession()
    {
        if(session_id() == '') {session_start();}// Start session if we dont already have one
        if (!isset($_SESSION['loggedin']) && $_SESSION['loggedin'] != true) {//Check to see if user is logged in
            header("location: ".Config::read('url.baseurl')); // User is logged in Redirect to homepage
        }else{
            //save session to this user
            $this->_userData->loggedin = $_SESSION['loggedin'];
            $this->_userData->provider = $_SESSION['provider'];
            $this->_userData->uid = $_SESSION['uid'];
            return true;
        }
    }
}

Game Class:

<?php
/**
 * Created by Base5 Designs.
 * and Dizzy Developments
 * User: Dizzy High
 * Date: 14/04/14
 * Time: 12:08
 */

require_once('config.class.php'); // require config class
require_once('dbconn.class.php'); //require database connector

/**
 * Class Game - instance of a game
 */
class Game {
    private $_conn; // database handler
    public $gameID; // id of game
    public $name; //name of game
    public $host; // uid of game host (the game creator)
    public $playerCount; // number of players in game
    public $roundTime; //round time of game in seconds
    public $visibility; // public (1) or private game  (0)
    public $alias; // whether game players use an alias name or not (1 yes 2 no)
    public $status; //status of game (waiting, active, complete)
    public $currentRound; // current round of game (0 not started, odd day, even night)
    public $startTime; // datetime - start time of game
    public $createdTime; // datetime date game created

    /**
     * Init class with Database Connection
     */
    function __construct(){
        $this->_conn = DBconn::getInstance(); //get DB instance
    }

    /**
     * assignSettings - assigns settings to game instance
     * @param $name - property name
     * @param $value - property value
     */
    function assignSetting($name, $value){
        $whitelist = array('id', 'name', 'host', 'playerCount', 'roundTime', 'visibility', 'alias', 'status', 'currentRound', 'startTime', 'createdTime');
        if (in_array($name, $whitelist)) {
            $property = $name;
            $this->$property = $value;
        }
    }

    /**
     * makeGame - creates a new game and adds it to the database with the defined settings
     * @param  array(): $settings- game settings for this game instance
     * @var $pdoQuery STRING
     * @var $pdoData array() - array of column and values to update
     * @var $PDOStatement
     * @return Game - this game instance
     * @throws PDOException - failed to run the pdo statement
     * @throws Exception  - throws the exception from the PDOException
     */
    function makeGame($settings){
        if (is_array($settings)) {
            foreach($settings as $k=>$v)  {
                $this->assignSetting($k, $v);
            }
        } else {
            die('Config Error!');
        }
        try{
            //add the game to the games table

            $pdoQuery = ('INSERT INTO games_gam (name_gam, host_gam, playercount_gam, roundtime_gam, visibility_gam, alias_gam) VALUE (:gameName, :gameHost, :playerCount, :roundTime, :visibility, :alias)');
            $pdoData = array(
                'gameName' => $this->name,
                'gameHost' => $this->host,
                'playerCount' => $this->playerCount,
                'roundTime' => $this->roundTime,
                'visibility' => $this->visibility,
                'alias' => $this->alias
            );
            $PDOStatement = $this->_conn->dbh->prepare($pdoQuery);
            $PDOStatement->execute($pdoData);
            $this->gameID = $this->_conn->dbh->lastInsertId();
            //add creator to game
            $this->addPlayer($this->host);
            return $this;
        }catch(PDOException $e){
            throw new Exception('There was an error creating a new game ', 0, $e);
        }
    }

    /**
     * getGame - queries the database and stores the results in this game instance
     * @param $gameID - the id of the game to fetch from the database
     * @var $pdoQuery STRING
     * @var $pdoData array() - array of column and values to update
     * @var $row PDORow
     * @return Game - this game instance
     * @throws PDOException - failed to run the pdo statement
     * @throws Exception  - throws the exception from the PDOException
     */
    function getGame($gameID){
        try{
            $pdoQuery = ('SELECT
                id_gam as `id`,
                name_gam as `name`,
                host_gam as `host`,
                playercount_gam as `playerCount`,
                roundtime_gam as `roundTime`,
                visibility_gam as `visibility`,
                status_gam as `status`,
                alias_gam as `alias`,
                starttime_gam as `startTime`,
                createdtime_gam as `createdTime`
                FROM games_gam
                WHERE id_gam = :id'
            );
            $PDOStatemnet = $this->_conn->dbh->prepare($pdoQuery);
            $PDOStatemnet->bindParam(':id', $gameID);
            $PDOStatemnet->execute();
            $row = $PDOStatemnet->fetch(PDO::FETCH_OBJ);
            $this->gameID = $row->id;
            $this->name = $row->name;
            $this->host = $row->host;
            $this->playerCount = $row->playerCount;
            $this->roundTime = $row->roundTime;
            $this->visibility = $row->visibility;
            $this->status = $row->status;
            $this->alias = $row->alias;
            $this->startTime = $row->startTime;
            $this->createdTime = $row->createdTime;
            return $this;
        }catch(PDOException $e){
            throw new Exception('There was an error getting the game: '.$gameID, 0, $e);
        }
    }

    /**
     * addPlayer - adds player to a game
     * @param $uid - uid of player
     * @return bool
     * @throws PDOException
     * @throws Exception
     */
    function addPlayer($uid){
        if(!$this->playerInGame($uid)){
            try{
                $pdoQuery = ('INSERT INTO gameplayers_gpl (idgam_gpl, uid_gpl) VALUE (:idgam, :uid)');
                $pdoData = array(
                    ':idgam' => $this->gameID,
                    ':uid' => $uid
                );
                $stmt = $this->_conn->dbh->prepare($pdoQuery);
                $stmt->execute($pdoData);
                return true;
            }catch(PDOException $e){
                throw new Exception('There was an error adding user '.$uid.' to the game', 0, $e);
            }
        }
    }

    /**
     * leaveGame - Removes a player from a game
     * @param $uid - the uid of the player
     * @throws Exception
     * @internal param $gameID - the id of the game
     * @return $this
     */
    function leaveGame($uid){
        if($this->playerInGame($uid)){
            try{
                $pdoQuery = ('DELETE FROM gameplayers_gpl WHERE uid_gpl = :uid AND idgam_gpl = :id');
                $pdoData = array(
                    ':uid' => $uid,
                    ':id'=> $this->$gameID
                );
                $stmt = $this->_conn->dbh->prepare($pdoQuery);
                $stmt->execute($pdoData);
                return $this;
            }catch(PDOException $e){
                throw new Exception('There was an error removing the user '.$uid.' from the game '.$this->gameID, 0, $e);
            }
        }
    }

    /**
     * gameFull - checks to see if a game has all player slots filled
     * @internal $pdoQuery STRING
     * @internal $PDOStatement PDOStatement
     * @throws Exception
     * @return bool true: game is full
     * @return bool false: has slots
     */
    function gameFull(){
        try{
            $pdoQuery = ('SELECT count(1) FROM gameplayers_gpl WHERE idgam_gpl = :gameID');
            $PDOStatement = $this->_conn->dbh->prepare($pdoQuery);
            $PDOStatement->bindParam(':gameID', $this->gameID);
            $PDOStatement->execute();
            $rowCount = (int) $PDOStatement->fetchColumn();
            return $rowCount < $this->playerCount;
        }catch(PDOException $e){
            throw new Exception('There was an error while checking if the game  '.$this->gameID.' was full', 0, $e);
        }
    }

    /**
     * playerInGame - checks to see if a player in the game
     * @param $uid: id of user
     * @internal @var $pdoQuery String
     * @internal @var $PDOStatement
     * @internal @var $pdoData array
     * @throws Exception
     * @return bool
     */
    function playerInGame($uid){
        try{
            $pdoQuery = ('SELECT count(1) FROM gameplayers_gpl WHERE idgam_gpl = :gameID AND uid_gpl = :uid');
            $PDOStatement = $this->_conn->dbh->prepare($pdoQuery);
            $pdoData = array(
                ':uid' => $uid,
                ':gameID'=> $this->gameID,
            );
            $PDOStatement->execute($pdoData);
            $rowCount = (int) $PDOStatement->fetchColumn();
            return $rowCount > 0;
        }catch(PDOException $e){
            throw new Exception('There was an error while checking if '.$uid.' is in game  '.$this->gameID, 0, $e);
        }
    }

    /**
     * @param $roundtime int - roundtime in seconds
     * @return array
     * @throws Exception
     */
    function getGamesList($roundtime){
        try{
            $pdoQuery = ('SELECT
                id_gam as `id`,
                name_gam as `name`,
                host_gam as `host`,
                playercount_gam as `playerCount`,
                roundtime_gam as `roundTime`,
                visibility_gam as `visibility`,
                status_gam as `status`,
                alias_gam as `alias`,
                starttime_gam as `startTime`,
                createdtime_gam as `createdTime`
                FROM games_gam
                WHERE visibility_gam = :visibility AND roundtime_gam = :roundtime AND status_gam = :gamestatus'
            );
            $PDOStatement = $this->_conn->dbh->prepare($pdoQuery);
            $pdoData = array(
                'visibility' => 0,
                'roundtime' => $roundtime,
                'gamestatus' => 'waiting',
            );
            $PDOStatement->execute($pdoData);
            return $PDOStatement->fetchAll();
        }catch(PDOException $e){
            throw new Exception('There was an error while getting the games list  ', 0, $e);
        }

    }
    /**
     * Convert number of seconds into years, days, hours, minutes and seconds
     * and return an string containing those values
     * the value of seconds could be greater than 1 day so unable to use phps datetime function
     * @param integer $seconds: Number of seconds to parse
     * @return string
     */
    function secondsToTime($seconds){
        $y = floor($seconds / (86400*365.25));
        $d = floor(($seconds - ($y*(86400*365.25))) / 86400);
        $h = gmdate('H', $seconds);
        $m = gmdate('i', $seconds);
        $s = gmdate('s', $seconds);

        $string = '';

        if($y > 0){
            $yw = $y > 1 ? ' years ' : ' year ';
            $string .= $y . $yw;
        }
        if($d > 0){
            $dw = $d > 1 ? ' days ' : ' day ';
            $string .= $d . $dw;
        }
        if($h > 0){
            $hw = $h > 1 ? ' hours ' : ' hour ';
            $string .= $h . $hw;
        }
        if($m > 0){
            $mw = $m > 1 ? ' minutes ' : ' minute ';
            $string .= $m . $mw;
        }
        if($s > 0){
            $sw = $s > 1 ? ' seconds ' : ' second ';
            $string .= $s . $sw;
        }

        return preg_replace('/\s+/',' ',$string);
    }

    function getHost($gameID){

    }
    function initGame(){

    }

    function processVotes(){

    }

    function setPlayerRole($uid, $role){


    }

    function getPlayerRole(){

    }

    function getPlayerHealth(){

    }

    function getGameDay(){

    }

    function getGameVotes(){

    }

for completness heres my database singleton, i have read this is a bad idea and some say its ok.

<?php
//php class for connecting to database

require_once('config.class.php');
class DBConn
{   
    public $dbh; // handle of the db connexion
    private static $instance;

    private function __construct()
    {

        // building data source name from config
        $dsn =  Config::read('db.dsn');
        // getting DB username from config
        $user = Config::read('db.user');
        // getting DB password from config                
        $password = Config::read('db.password');
        // set database handler
        $this->dbh = new PDO($dsn, $user, $password);
    }

    /**
     * getInstance
     * creayes a singleton of a pdo connection
     * @return DBConn
     */
    public static function getInstance()
    {
        if (!isset(self::$instance))
        {
            $object = __CLASS__;
            self::$instance = new $object;
        }
        return self::$instance;
    }
}

really i think the way i need to go is to do as you suggest and create a Database layer class that connects to my db, then perhaps extend this class with any of my functions that do reading and writing to the db, there's likely to be a lot of these methods so maybe its going to make it too big??

something along the lines off:

<?php
//php class for connecting to database
class DBLayer
{
    public $dbh; // handle of the db connexion

    private function __construct($config)
    {
        // building data source name from config
        $dsn =  $config->read('db.dsn');
        // getting DB username from config
        $user = $config->read('db.user');
        // getting DB password from config                
        $password = $config->read('db.password');
        // set database handler
        $this->dbh = new \PDO($dsn, $user, $password);
    }

    public function getConnection()
    {
        if ($this->dbh)
        {
            return $this->dbh;
        }
        return false;
    }
}

class GameLayer EXTENDS DBLayer
{

}

This extended class would maybe need to have the current user and current game injected into it via dependency injection, the DBLayer already has a dependency is it possible to inject other dependencies into the GameLayer class?

\$\endgroup\$
  • \$\begingroup\$ At a quick glance, it looks a lot neater. Two more things that I notice right off the bat, learn what dependency injection is all about, and only apply comments you need. Comments such as // getting DB password from config only duplicate something the code itself easily explains. \$\endgroup\$ – Alex L Apr 20 '14 at 22:07

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.