3
\$\begingroup\$

I made this code for simple leveling game.

I think this code may have some issues with security. Also, are there some way to make this code more efficient and reliable?

For level scaling I will use this method. Should I make lvl, xp and nxtlevel variables to array or is that a bad idea?

class LevelingSystem
{

  private $_lvl;
  private $_xp;
  private $_nxtlevel;

  function __construct($lvl=1,$xp=0,$nxtlevel=30)
  {

    $this->_lvl=$lvl;
    $this->_xp=$xp;
    $this->_nxtlevel=$nxtlevel;
  }

  function setxp($setxp)
  {
    global $db;
    $stmt = $db->prepare("UPDATE user_atri SET totalxp =totalxp+".$setxp." WHERE id=1");
    $updatexp=$stmt->execute();
    return $updatexp;
  }

  private function getxp()
  {
    global $db;
    $query = $db->prepare("SELECT totalxp FROM user_atri where id=1");
    $query->execute();
    $totalxp=$query->fetch(PDO::FETCH_ASSOC);
    $this->_xp=$totalxp["totalxp"];
    return $totalxp["totalxp"];
  }

  function lvl()
  {

    $this->getxp();
    while ($this->_xp>=$this->_nxtlevel)
    {
        $this->_lvl+=1;
        $this->_xp =$this->_xp - $this->_nxtlevel;
        $this->_nxtlevel = round($this->_nxtlevel * 1.6);
    }

    //$levelprint=sprintf("Youre level is: %s ",$this->_lvl);
    //echo $this->_xp;
    //echo "<br>";
    //return $levelprint;
    return $this->_lvl;

  }

}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

First: in the book 'Refactoring' by Martin Fowler, one of the code smells is "Obsession with elementary types". And the advice is to create/take out variables and move them to another class.

Array is an elementary type. The resolution is to create an entity class Settings and set member variables $lvl,$xp,$nxtlevel.

Second: global $db; transfer through object.

Using global variables is bad practice. If you want to use a Database you should create an object for Database access. Best practice use DI. For PHP one such library is php-di.

Your code may look like this:

<?php

  class LevelingSystem {

  private $settings;
  private $db;

  function __construct(SettingsEntity $settings, DBInterface $db ){
    $this->settings=$settings;
    $this->db = $db;
  }

  public function setxp($setxp) {
    $result =  $this
                ->db
                ->prepare("UPDATE user_atri SET totalxp =totalxp+" . $setxp . " WHERE id=1")
                ->execute();

    if(!$result) {
       throw new Exception('DB error');  //you can send your exception 
    }        

    return $this;
  }

  public function calcSettings() {
   $xp = $this->getxp();
   $nxtlevel = $this->settings->getNxtlevel();
   $lvl = $this->settings->getLvl();

   while ($xp >= $nxtlevel) {
        $lvl += 1;
        $xp = $xp - $nxtlevel;
        $nxtlevel = round($nxtlevel * 1.6);
    }

     $this->settings
     ->setlvl($lvl)
     ->setXp($xp)
     ->setNxtlevel($nxtlevel);

     return $this;
  }

  public function getLvl() {
      return $this->settings->getLvl();
  }

  private function getxp() {
    $totalxp = $db
                ->prepare("SELECT totalxp FROM user_atri where id=1")
                ->execute()
                ->fetch(PDO::FETCH_ASSOC);

    if(!isset($totalxp["totalxp"])) {
       throw new Exception('Some DB error'); //you can send your exception 
    } 

    return $totalxp["totalxp"];
  }
}
\$\endgroup\$
5
  • \$\begingroup\$ I didn't quite understand how my Settings class should look like. \$\endgroup\$
    – Godhaze
    Aug 29, 2017 at 8:48
  • \$\begingroup\$ class SettingsEntity { public $_lvl; public $_xp; public $_nxtlevel; function __construct($lvl=1,$xp=0,$nxtlevel=30) { $this->_lvl=$lvl; $this->_xp=$xp; $this->_nxtlevel=$nxtlevel; } function setXp($xp) { $this->_xp=$xp; } function setlvl($lvl) { $this->_lvl=$lvl; } function setNxtlevel($nxtlevel) { $this->_nxtlevel=$nxtlevel; } function getLvl() { return $this->_lvl; } function getNxtlevel() { return $this->_nxtlevel; } } \$\endgroup\$
    – Godhaze
    Aug 29, 2017 at 8:48
  • \$\begingroup\$ this what I made, but it aint working \$\endgroup\$
    – Godhaze
    Aug 29, 2017 at 8:48
  • \$\begingroup\$ nvm, got it working, I forgot to add return $this to set functions \$\endgroup\$
    – Godhaze
    Aug 29, 2017 at 9:23
  • \$\begingroup\$ add call chain. For example in SettingsEntity method setlvl most return $this. all Setters most return $this. setlvl($lvl) { $this->_lvl=$lvl; return $this;} \$\endgroup\$ Aug 29, 2017 at 14:02

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.