ОО авт Либ - на правильном пути?


class User {

    protected $DBH;
    protected $STH;

    public function __construct($DBH) {
        $this->DBH = $DBH;
    }

    public function logged_in() {
        if (isset($_SESSION['userid']) && isset($_SESSION['hash'])) {
            $hash = sha1($_SESSION['userid'] . $_SERVER['HTTP_USER_AGENT']);

            if ($_SESSION['hash'] == $hash)
                return true;
        }
        return false;
    }

    public function login($username, $password) {
        $password = sha1($password);
        $this->STH = $this->DBH->prepare("SELECT id, banned, activated FROM users WHERE username = ? AND password = ?");
        $this->STH->setFetchMode(PDO::FETCH_OBJ); 
        $this->STH->execute(array($username, $password));

        if ($this->STH->rowCount() > 0)
            return $this->STH->fetch();
    }

    public function create_account($username, $password, $email) {
        $password = sha1($password);
        $activation_key = md5($username . $email);

        $this->DBH->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

        try {
            $this->STH = $this->DBH->prepare("INSERT INTO users(username, password, email, activation_key, created) VALUES(:username, :password, :email, :activation_key, :created)");
            $this->STH->bindParam(':username', $username);
            $this->STH->bindParam(':password', $password);
            $this->STH->bindParam(':email', $email);
            $this->STH->bindParam(':activation_key', $activation_key);
            $this->STH->bindParam(':created', time());
            $this->STH->execute();
        } catch (PDOException $e) {
            //$e->getMessage();
            return false;
        }

        return true;
    }

    public function check_username($username) {
        $this->STH = $this->DBH->prepare("SELECT username FROM users WHERE username = ?");
        $this->STH->execute(array($username));

        if ($this->STH->rowCount() > 0)
            return false;

        return true;
    }

    public function check_email($email) {
        $this->STH = $this->DBH->prepare("SELECT email FROM users WHERE email = ?");
        $this->STH->execute(array($email));

        if ($this->STH->rowCount() > 0)
            return false;

        return true;        
    }

    public function activate($key) {
        $this->STH = $this->DBH->prepare("SELECT activation_key FROM users WHERE activation_key = ?");
        $this->STH->execute(array($key));

        if ($this->STH->rowCount() > 0) {
            $this->STH = $this->DBH->prepare("UPDATE users SET activated = 1, activation_key = null WHERE activation_key = ?"); 
            $this->STH->execute(array($key));

            return true;
        }

        return false;
    }  
}

Я делаю это право? Что я должен изменить? Добавить и т. д. Я должен использовать больше попробовать { } блоков catch/и т. д. Дать мне несколько советов о том, как улучшить код. (Лучше производительность и т. д.).



192
1
php
задан 30 ноября 2011 в 04:11 Источник Поделиться
Комментарии
1 ответ


  • Что касается попробовать/поймать: это зависит от вашего пользовательского класса может обрабатывать ошибки или нет.


    • Исключение вы не можете справиться, например, если SQL-оператор содержит синтаксическую ошибку или SQL-сервера. Вы не можете ничего поделать программно во время выполнения, за исключением отображения сообщения об ошибке для пользователя. В этом случае я бы не окружить ДБ-код с try/catch, но реализовать глобальную попробовать/поймать который не просто отображать сообщение об ошибке для пользователя и журнал ошибок для разработчиков.

    • Но если вы исключение, потому что, например, ваше имя пользователя-поле является уникальными кто-то пытается вставить имя пользователя, которое уже существует, вы могли бы решить эту проблему автоматически, просто добавив номер имя пользователя и повторите попытку вставить (ладно, это глупый пример, но надеюсь суть попадается). Псевдокод для этой тупой пример может быть как:

      $statement = $db->prepare("INSERT INTO user (username, ....) values (?, ...)");
      $tryInsert = true;
      $tries = 0;
      while($tryInsert) {
      $insertName = $username;
      if($tries > 0) {
      $insertName .= $tries;
      }
      try {
      $statement->execute(array($insertName, ...));
      $tryInsert = false; // it worked, stop trying
      } catch(PDOException $e) {
      if($e->code() == "23000") {
      // 23000 means unique constraint violation, probably
      // duplicate username
      ++$tries;
      } else {
      // we can't handle the exception, therefore we just re-throw it
      throw $e;
      }
      }
      }

    Я думаю, что в вашем коде нет никаких исключений, которые вы могли бы справиться. Если исключение происходит, это скорее всего из-за некоторой проблемы с SQL-сервер или SQL-оператор является неправильным. В этом случае это не имеет смысла, чтобы обернуть код в try/поймать. Но вы должны добавить какой-то глобальный обработчик исключений, который показывает сообщение об ошибке (типа: Ой что-то пошло не так, разработчики сообщили).


  • В входа изменить две последние строки:

    if (($row = $this->STH->fetch()) !== false)
    return $row;

    Проблема с использования pdostatement::recordCount() , что это не портативный для запросов Select и приведенный выше код делает то же самое, как ваш код.


  • Затем в create_account, вы используете ПДО::метод setAttribute изменить ПДО ошибка режиме поставляемого ПДО-экземпляр. Изменить это так, что это называется, когда ПДО инициализируется и не глубоко в какой-маппер-класса.

  • Код активации метод может быть переписан, чтобы просто выполнить один запрос:

    public function activate($key) {
    $this->STH = $this->DBH->prepare("UPDATE users SET activated = 1, activation_key = null WHERE activation_key = ?");
    $this->STH->execute(array($key));

    // rowCount is set to the number of rows affected by UPDATE
    return $this->STH->rowCount() > 0;
    }


  • Необязательно: это может быть упрощена:

    if (isset($_SESSION['userid']) && isset($_SESSION['hash'])) {

    К этому:

    if (isset($_SESSION['userid'], $_SESSION['hash'])) {

  • Дополнительно: рекомендуется использовать phpass для хэширования паролей. Это, безусловно, более безопасной, что просто в SHA1 с солью бросали в класс и прост в использовании. Если вы делаете это, вы должны переработать свой логин способ немного (имеется в виду пароль проверки должно быть сделано в PHP при использовании phpass).

  • Если вы хотите изменить свой check_*-методы вы могли бы сделать нечто подобное:

    protected function check_record($field, $value) {
    $this->STH = $this->DBH->prepare("SELECT COUNT(*) FROM users WHERE {$field} = ?");
    $this->STH->execute(array($value));

    $count = $this->STH->fetchColumn();

    if($count !== false) {
    return (int)$count > 0;
    }

    return null;
    }

    public function check_username($username) {
    return $this->check_record("username", $username);
    }

    public function check_email($username) {
    return $this->check_record("email", $username);
    }


1
ответ дан 30 ноября 2011 в 07:11 Источник Поделиться