2
\$\begingroup\$

Currently, I'm working on gamification platform and I have the following rules for achieving a new level:

  • Level 0: When user registers
  • Level 1: When user confirms account
  • Level 2: When user completes a quiz
  • Level 3: When user completes a mission and publish more than 5 comments in the blog
  • Level 4: When user completes more than two missions and completes a campaign (campaign means donating money for something)
  • Level 5: Raised at least $50 in campaigns and complete at least two campaigns

Now, I have the following database where I'll keep a track of each user action (e.g. complete_quiz, complete_mission, etc.):

Achievements

  • id (int)
  • event (varchar)
  • amount (int) # used when event has specific value, e.g. money for complete_campaign event
  • user_id (int)
  • date_created (int)

When the user make any action (e.g. complete_mission), I'll insert the action in the table above and will call the following method to check if user has new level unlocked:

public function checkIfAchievmentUnlocksNewLevel($userObj)
{       
    $currentLevel = $userObj->level;

    $nextLevel = ++$currentLevel

    $isLevelUnlocked = false;

    switch($nextLevel)
    {
        case 0:
        case 1:
            // Each registered user will have these levels by default.
        break;
        case 2:
            $completedQuiz = $this->db->select("SELECT " . $this->fieldsList . " FROM " . $this->table . " WHERE event = :event AND user_id = :user_id ", array(':event' => 'complete_quiz', ':user_id' => $userObj->id));
            $isLevelUnlocked = count($completedQuiz) ? true : false;
        break;
        case 3:
            $completedMissions = $this->db->select("SELECT " . $this->fieldsList . " FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_mission', ':user_id' => $userObj->id));
            $wildwireComments = $this->db->select("SELECT " . $this->fieldsList ." FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'wildwire_comment', ':user_id' => $userObj->id));              
            $isLevelUnlocked = (count($completedMissions) && count($wildwireComments) >= 5) ? true : false;
        break;
        case 4:
            $completedMissions = $this->db->select("SELECT " . $this->fieldsList . " FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_mission', ':user_id' => $userObj->id));
            $completedCampaigns = $this->db->select("SELECT " . $this->fieldsList ." FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_campaign', ':user_id' => $userObj->id));
            $isLevelUnlocked = (count($completedMissions) >= 2 && count($completedCampaigns)) ? true : false;
        break;
        case 5:
            $campaignRaisedMoney = $this->db->select("SELECT SUM(amount) FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_campaign', ':user_id' => $userObj->id));
            $completedCampaigns = $this->db->select("SELECT " . $this->fieldsList ." FROM " . $this->table . " WHERE event = :event AND user_id = :user_id", array(':event' => 'complete_campaign', ':user_id' => $userObj->id));
            $isLevelUnlocked = (count($compaignRaisedMoney) >= 50.00 && count($completedCampaigns) >= 2) ? true : false;
        break;
    }

    if($isLevelUnlocked) 
    {
        $userHelper->setLevel($nextLevel);
    }

}

This code is working now, but I want to refactor it and am looking for any suggestions on how to improve it.

\$\endgroup\$
2
  • \$\begingroup\$ I'd recommend adding an XP system instead. 100xp to level up and for example mail verification gives 100xp, every donation gives 2xp etc. \$\endgroup\$
    – ave
    Jun 19, 2016 at 20:36
  • \$\begingroup\$ Yes, I though about it, but I'll need to make the same changes (am I wrong?) in order to check if user has new level unlocked. For example, take a look at "Level 4", from publishing comments in blog, user can ear a lot of points... but that doesn't mean that next level will be unlocked... \$\endgroup\$ Jun 20, 2016 at 5:07

2 Answers 2

1
\$\begingroup\$

First, I am concerned about the underlying data model. I am not sure that trying to fit all different event types into the same table makes sense here since the events are so different in nature (account validation vs. taking a quiz vs. making comments vs. completing missions vs. completing a campaign).

My guess is that you should have separate tables in the database for each of these types of events. One table for quiz results for all users, one table for all user comments, one table for storing mission information, etc.

Second, I am concerned with how you are hard-coding the level requirements into this section of code and storing the level on the user record. This is a very tight coupling of your leveling logic with the user object (presumably in database as well). What happens if you change your leveling criteria?

I would probably strive to store necessary properties or expose necessary methods with the user class to be able to pass a user object to an independent level-determination class where it is compared against levelling criteria. So to fill out a rough class skeleton, perhaps you are looking at something like this:

class User {
    public $id;
    public $name;
    // etc.

    public function __construct($id) {
        // set up object from database record
    }
    public function accountConfirmed() {
        // return true/false as to whether account is confirmed
    }
    public function getComments() {
        // get list of all comment objects
        // perhaps pass some parameter to allow for filtering
    }
    public function getQuizzes() {
        // get list of all quizzes, and perhaps have parameters for filtering
    }
    public function getMissions() {
        // get missions
    }
    public function getCampaigns() {
        // get campaigns
    }
    public function getCampaignTotals() {
        // get campaign totals
    }
    public function setLevel() {
        // place to set the level on the user
        // perhaps you update this to the database if changed
        // and you decide you do need to store level on the user record for ease of lookup
    }
}

class userLevelCalculator {
    // place to store user object that was passed to class
    protected $user;
    protected $levelCheckFunctions = array();

    // constructor receives user object
    public function __construct(User &$user) {
        $this->user = $user;
    }

    // logic to determine user level
    // this could probably be broken down into separate class methods if needed
    public function getUserLevel() {
        // check for level 1
        if(!$user->accountConfirmed()) {
            $user->setLevel(0);
            return 0;
        }
        // check for level 2
        $quizCount = $user->getQuizzes();
        if($quizCount < 1) {
            $user->setLevel(1);
            return 1;
        }
        // check for level 3  
        $commentCount = $user->getComments();
        $missionCount = $user->getMissions();
        if($commentCount < 5 || $missionCount < 1) {
            $user->setLevel(2);
            return 2;
        }
        // check for level 4
        $campaignCount = $user->getCampaigns();
        if($missionCount < 2 || $campaignCount < 1) {
            $user->setLevel(3);
            return 3;
        }
        // check for level 5
        $campaignTotals = $user->getCampaignTotals();
        if($campaignCount < 2 || $campaignTotals < 50) {
            $user->setLevel(4);
            return 4;
        }
        $user->setLevel(5);
        return 5;
    }    
}
\$\endgroup\$
2
\$\begingroup\$

I would agree with @ardaozkal that an XP based system would be better (more flexible, etc).

But assuming that your approach is good enough for your situation, I also have some suggestions:

  • You have quite a bit of duplication, for example the completed mission queries or the completed campaign queries. You should extract those to their own function and reuse that.
  • Your code could generally benefit from more functions. Other candidates would be hasCompletexXMissions($db, $amount) or getCompletedMissionCount($db).
  • If this project is a bit more complex, you definitely want to add classes, such as Mission, which may then contain the functions mentioned above.
  • Your lines are too long, which seriously impacts readability. 226 characters don't even fit on many modern screens. I would aim for about 80 characters.
  • You have a bit too much vertical whitespace for my taste. Do the first three statements really all need their own paragraph?
  • You can get rid of $currentLevel, it's only used in one location, and the name doesn't really add any value. Just use $userObj->level directly.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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