HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

A class to add and select players within a database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
playerswithindatabaseandselectclassadd

Problem

I have a question and want to know if my class is considered object oriented.

class Player {

    private $db

    function __construct($db) {

        $this->db = $db;

    }

    public function set_player($name, $position, $number, $team) {

        try {

            $sql = 'INSERT INTO players SET playername = :playername, position = :position, teamid = :teamid, jersey = :jersey';
            $this->db->prepare($sql);
            $s->bindValue(':playername', $name);
            $s->bindValue(':teamid', $team);
            $s->bindValue(':position', $position);
            $s->bindValue(':jersey', $number);
            $s->execute();

        } catch (PDOEXception $e) {

            echo 'Could not add player, please try again.';
            exit();

        }

    }

    public function get_player($id) {

        $result = $this->db->query('SELECT * FROM players WHERE id =' . $id);
        $row = $result->fetch(PDO::FETCH_OBJ);
        return $row;

    }

}


In another file I will use this class in this way

include_once('./includes/global.php');

$player = new Player($pdo);

$player->set_player(Chris, Forward, 13, 1);

echo $player->get_player(1)->playername;
echo $player->get_player(1)->position;

Solution

Calling get_player twice is not a good idea, as each time a db query is necessary.

As you are asking about OOP: You could add an actual Player object, which contains playername, position, etc as fields. Your current class could then be renamed PlayerDAO or something, and would then return a player object.

The main advantage would be that a concrete object is easier to use than an anonymous one (classes are to some degree self-documenting, while you can't be sure what an anonymous object actually contains).

Misc

  • don't die or echo in classes, it makes code less flexible and harder to reuse, as the calling code cannot try to recover.



  • get_player and set_player could be renamed to get and set as the class is already called player.



  • It would probably also be good to rename set to save or similar, as it makes more explicit that there is some kind of database interaction (set sounds as if a private field is set).

Context

StackExchange Code Review Q#115034, answer score: 4

Revisions (0)

No revisions yet.