@tryvols
Front-End разработчик

Насколько код ООП и что бы вы посоветовали по его улучшению?

Недавно начал разбираться в ООП и MVC, по этому очень хочу узнать на правильном ли я пути и как его еще больше выпрямить) Не буду многословен, вот моя первая модель:

class Model_admin extends Model
{

	private $name;
	private $password;
	private $code;
	private $mail;

	public function get($login, $password) {

		$this->name = $login;
		$this->password = $password;

		$this->db_connect();

		$result = msql_query("SELECT 'login', 'password', 'mail' FROM 'admin_authorization'";
		$arr = mysql_fetch_array($result);

		do {

			if ($login === $arr['login']) {

				if (
				    md5( md5( trim( $password ))) === $arr['password']
				) {
					$this->mail = $arr['mail'];
					$this->random_code();
					return true;
				}

			}

		} while ($arr = mysql_fetch_array($res));

		return false;

	}

	private function random_code($length = 15) {

		$symbols = '0123456789abcdefghijklmnopqrstuvwxyz_-~!+*%$#&';

		for ($i = 0; $i < (int)$length; $i++) 
		{
			$num = rand (1, strlen ($symbols));
			$this->code .= substr ($symbols, $num, 1);  
		}

		$bool_update = mysql_query('UPDATE "admin_authorization" SET code="'.$this->code.'" WHERE login="'.$this->name.'"');

		if ($bool_update) $this->send_code();

	}

	private function send_code() {

		mail ($this->mail, "Admin code", $this->code);
	}

	public function check_code($user_code) {

		$this->code = mysql_query("SELECT 'code' FROM admin_authorization WHERE code='".$user_code."'");

		if ($this->code != '') return true;
	}

}
  • Вопрос задан
  • 681 просмотр
Пригласить эксперта
Ответы на вопрос 6
index0h
@index0h
PHP, Golang. https://github.com/index0h
<?php
// PSR-1, PSR-2, PSR-4 Читаем и пользуем!
// namespace все дела... "Model_admin" - это прошлое.
// phpDocumentor - твой друг, прописывай всюду типы данных
class Model_admin extends Model
{
// Лишний перевод строки
    private $name;
    private $password;
    private $code;
    private $mail;
// Не информативное название. get model admin... что бы это значило...
    public function get($login, $password) {
// Где проверка аргумантов? Влететь может что угодно
        $this->name = $login;
        $this->password = $password;
// Модель НЕ должна управлять подключением к БД, это должно выполняться выше в коде
        $this->db_connect();
// Код вообще проверялся?)) у вас закрывающей строки нет.
        $result = msql_query("SELECT 'login', 'password', 'mail' FROM 'admin_authorization'";
// сие уже deprecated, забудьте про mysql_*** функции, используйте PDO
        $arr = mysql_fetch_array($result);
// Зачем нужен цикл, это дро*ба БД!!! Вытягиваете одну запись по логину и проверяете соответствует ли пароль
        do {
// Лишний перевод строки
            if ($login === $arr['login']) {
// Лишний перевод строки
                if (
                    md5( md5( trim( $password ))) === $arr['password']
                ) {
                    $this->mail = $arr['mail'];
                    $this->random_code();
// Перед return лучше делать перевод строки
                    return true;
                }
// Лишний перевод строки
            }
// сие уже deprecated, забудьте про mysql_*** функции, используйте PDO
// Присваивание в условиях управляющих кнострукций лучше не делать, это операции разного характера
        } while ($arr = mysql_fetch_array($res));

        return false;
// Лишний перевод строки
    }

    private function random_code($length = 15) {
// Где проверка аргумантов? Влететь может что угодно
        $symbols = '0123456789abcdefghijklmnopqrstuvwxyz_-~!+*%$#&';

        for ($i = 0; $i < (int)$length; $i++)
        {
            $num = rand (1, strlen ($symbols));
            $this->code .= substr ($symbols, $num, 1);
        }
// сие уже deprecated, забудьте про mysql_*** функции, используйте PDO
        $bool_update = mysql_query('UPDATE "admin_authorization" SET code="'.$this->code.'" WHERE login="'.$this->name.'"');
// Вот так писать плохо, всегда используйте фигурные скобки.
        if ($bool_update) $this->send_code();
// Лишний перевод строки
    }

    private function send_code() {
// Модель НЕ должна отправлять письма, под отправку обычно пишется отдельная подсистема/сервис
        mail ($this->mail, "Admin code", $this->code);
    }

    public function check_code($user_code) {
// Где проверка аргумантов? Влететь может что угодно
// SQL инъекция!!!!
// сие уже deprecated, забудьте про mysql_*** функции, используйте PDO
        $this->code = mysql_query("SELECT 'code' FROM admin_authorization WHERE code='".$user_code."'");
// Вот так писать плохо, всегда используйте фигурные скобки.
        if ($this->code != '') return true;
    }
// Лишний перевод строки
}
Ответ написан
Fesor
@Fesor
Full-stack developer (Symfony, Angular)
забейте на время на MVC, почитайте про SOLID.

1) mysql_* функции депрекейтед, используйте pdo (mysqli слишком низкоуровневая штука)
2) делать 2 раза md5 не имеет никакого смысла. Сейчас можно генерить миллиарды хэшей в секунду на видеокартах так что подбор коллизии к хэшу займет не сильно много времени и большая часть паролей будет подобрана за первые пару часов. Используйте password_hash и password_verify. Для PHP < 5.5 есть флэбэк написанный на PHP.
3) вы нарушили принцип единой ответственности, это к вопросу о том насколько ваш код ООП. Ну и принцип инверсии зависимостей туда же....
Ответ написан
@dk-web
Привет, коллега! Сейчас тебе посыпется, надеюсь... Мой совет - не бойся и прислушайся... я сначала не воспринимал все эти новшества, понадобилось время.
Но на правах первого ответа... если уж ООП стал постигать - сразу внедряй PDO и подготовленные запросы...
- уходи с mysql_query,,,
mysql_query("SELECT 'code' FROM admin_authorization WHERE code='".$user_code."'");
вот за это точно по голове "получишь")...
Ответ написан
oOLokiOo
@oOLokiOo
PHP Developer
используйте PDO, да
ну и логику разносите
в моделе не должно быть разных "SELECT * FROM", раз вы уже про ООП заговорили
делайте отдельный класс DB и используйте его аля - $DB->getOne('TABLE', array(PARAMS))
ну и про singleton можете почитать за одно
Ответ написан
trevoga_su
@trevoga_su
почитай фаулера для начала
Ответ написан
Комментировать
Acuna
@Acuna
Заполнил свой профиль
Совсем по мелочам если - все переменные можешь ввести в одном типе через запятую:

private $name, $password, $code, $mail;

Для работы с БД (любыми) нужно пользовать готовые библиотеки, свои или сторонние. Они регулярно развиваются и поддерживаются, не нужно городить свои велосипеды. Это же касается и работы с мылом, для него в PHP имеется мощнейшая библиотека phpmailer.
Остальное все коллеги более чем подробно расписали, нет смысла повторяться.
Ответ написан
Комментировать
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы
YCLIENTS Москва
от 200 000 до 350 000 ₽
Ведисофт Екатеринбург
от 25 000 ₽
ИТЦ Аусферр Магнитогорск
от 100 000 до 160 000 ₽
25 апр. 2024, в 12:23
2500 руб./за проект
25 апр. 2024, в 12:21
10000 руб./за проект