patternphpMinor
A class to add and select players within a database
Viewed 0 times
playerswithindatabaseandselectclassadd
Problem
I have a question and want to know if my class is considered object oriented.
In another file I will use this class in this way
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
As you are asking about OOP: You could add an actual
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
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_playerandset_playercould be renamed togetandsetas the class is already calledplayer.
- It would probably also be good to rename
settosaveor similar, as it makes more explicit that there is some kind of database interaction (setsounds as if a private field is set).
Context
StackExchange Code Review Q#115034, answer score: 4
Revisions (0)
No revisions yet.