Взаимодействие контроллера и модели?

Question

[Uber Noob]

Функция авторизации из контроллера:

// Авторизовать юзера с помощью электронной почты
    public function maillogin()
    {
        $POST = $_POST;
        $POST['mail'] = trim($POST['mail']);

        $validate = new Validator();

        if(!empty($_POST['mail']) && !empty($_POST['pwd']) && $validate->correctMail($_POST['mail']) === true){

			// В модель ничего не передаётся т.к. она возьмёт данные из POST
            $data = $this->userModel->{__FUNCTION__}();

            if($data === 'WrongPassword'){
                $view = new ErrorView();
                $view->wrongPwd();
            }
			elseif($data === 'WrongLogin') {
                $view = new ErrorView();
                $view->wrongLogin();
            }

        } else {
            $view = new ErrorView();
            $view->noData();
        }
    }

Фрагмент этой же функции из модели:

if(password_verify($POST['pwd'], $pwd[0]['pwd'])){
        $_SESSION['ok'] = 'Вы успешно авторизованы';
        header("Location: {$_SERVER['HTTP_REFERER']}");
        die();
    }

Вопросы по этому коду:
1. Метод maillogin() существует и в контроллере и в модели, нормально ли это, или лучше сделать mailloginController() и mailloginModel()
2. Если пользователь залогинится успешно, то метод в модели как видно не возвращает true, там же в модели создаётся сессия и юзер отправляется на страницу откуда пришёл. Это плохо? Надо вернуть true и редиректить в контроллере?
3. Создав экземпляр класса new ErrorView() заранее, например, там же где new Validator() я смогу сократить код ниже, а не писать создание экземпляра в каждом if но если всё пройдёт ок, то он потребуется и это будет напрасная трата памяти. Как лучше делать в этом случае?
4. die() в модели мне не нравится, но чем заменить его не знаю, как обычно делают в таких случаях?

Ну и если любые другие замечания будут - рад услышать.

Answer 1

[Дмитрий]

1. Нормально. ООП для того и создан, что метод с одним названием может быть в разных классах, но работать по своему, если это согласуется с общей логикой.

2. По хорошему модель должна просто описывать данные, в крайнем случае возвращать связанные с нею наборы данных, но не возвращать экземпляры сторонних классов

3. Не тот случай, где надо заботиться о трате памяти. Вы же не 10000 экземпляров создаете. Но можно было бы сделать красиво

new ErrorView($data)

чтобы конструктор errorView возвращал представление по имени ошибки, а функции wrongLogin и wrongPwd были бы приватными.

4) А зачем там вообще die?

Comments

[Uber Noob]

Спасибо за ваши замечания, чуть позже отмечу решением:

1, Сейчас при поиске в проекте maillogin() находится 2 совпадения, я хочу видеть при поиске только уникальный объект и место где оно вызывается.

2. Модель сейчас ничего не возвращает. Я говорю что можно сделать что бы она вернула просто true (данные) о возврате экземпляров сторонних классов речи не идёт.

3. По поводу new ErrorView($data) ErrorView один, а методов в нём много, на разные типы ошибок, сделав так мне придётся разруливать это всё где? В конструкторе ErrorView?

4. Потому что без return или exit метод продолжит выполнение до конца.

Answer 2

[Иван Корюков]

1) Вы пишете что модель сама берёт данные из post. Зачем тогда вам контроллер? Это плохая модель, она зависит от способа передачи данных . Контроллер должен получать данные и передавать их модели. А методы должны называться в соответствии с тем что они делают. Могут называться и одинаково, тут вопрос не к названию, а к содержимому.
2) то же самое. Модель должна вернуть данные, а уже контроллер дальше положит их куда надо.
3) подумайте сколько раз за один http-запрос будет создан лишний объект. Если это количество само по себе велико или зависит от обрабатываемых данных (например по одному лишнему объекту на выводимый на странице комментарий пользователя) то это плохо. Если один раз, то зависит от способа создания объекта. Если в конструкторе выполняются ресурсоёмкие операции, то... Это само по себе плохо. Конструктор не должен содержать кода (с некоторыми допущениями)
4) в третий раз повторяю - модель должна вернуть значение. Прервать выполнение программы она может в одном случае - возникла ошибка, такая что дальше работать бессмысленно. Для этого есть исключения. Контроллер должен поймать исключении и завершить выполнение, а лучше показать сообщение - "ой, возникла ошибка".
Во всех других случаях модель делает return, управление передаётся контроллеру и уже он, если необходимо делает die().
Хотя само по себе использование die() чаще всего является костылём.

Comments

[Uber Noob]

Хмм, на, про первый пункт что-то не подумал, но глобальные массивы нельзя передавать напрямую в функцию/метод на сколько я помню, в этом случае придётся дублировать POST из глобальной в обычную переменную и уже её передавать вот так:

$POST = $_POST;
$data = $this->userModel->{__FUNCTION__}($POST);

[Иван Корюков]

Uber Noob, а зачем вам передавать содержимое post в модель? Метод maillogin(), судя по названию, должен принимать переданные пользователем логин, пароль, ну и может быть что-то ещё специфическое. Так и передавайте в модель отдельные параметры

$data = $this->userModel->maillogin($_POST["email"], $_POST["password"]);

[Иван Корюков]

Uber Noob, и да, вызов метода модели через {__FUNCTION__} вот совсем не имеет смысла.