search

AlertRED / php-tasks

Работа по стажировке в компании 2UP на Laravel
0 stars 0 forks source link

Задание 3 #3

Open AlertRED opened 5 years ago

mx2s commented 5 years ago

https://github.com/AlertRED/phpTasks/blob/0b14c3343da71a70ce7a9ada5659149470e126ab/routes/api.php#L17 не совсем логично - работу с группами лучше вынести в отдельный контроллер, это ко всем 5 роутам относится

mx2s commented 5 years ago

image код плохо читается - старайся следить за этим)

mx2s commented 5 years ago

при запросе POST /api/v0/users/group

"message": "Class 'App\\Http\\Controllers\\v0\\Validator' not found",

идеальный вариант это выносить валидацию в реквесты https://laravel.com/docs/master/validation#form-request-validation

mx2s commented 5 years ago

https://github.com/AlertRED/phpTasks/blob/0b14c3343da71a70ce7a9ada5659149470e126ab/app/Http/Controllers/v0/UserProfilesController.php#L108 переменная $result нигде не используется

https://github.com/AlertRED/phpTasks/blob/0b14c3343da71a70ce7a9ada5659149470e126ab/app/Http/Controllers/v0/UserProfilesController.php#L92 $v то же самое - когда валидацию выполняешь она выкинет exception автоматически - не нужно получать результат и проверять (но на счет того как ты сейчас валидацию делаешь я на 100% не уверен)

AlertRED commented 5 years ago

phpTasks/routes/api.php

Line 17 in 0b14c33

Route::post('users/group', 'v0\UserProfilesController@postGroup');

не совсем логично - работу с группами лучше вынести в отдельный контроллер, это ко всем 5 роутам относится

Да, я в следующей ветке это исправил почти сразу. Я видел что это не логично, просто сомневался, потому что ожидал, что об этом будет явно сказано в задании.

AlertRED commented 5 years ago

image код плохо читается - старайся следить за этим)

Хорошо, я что-то придумаю. Я так и не смог найти автоформатирования кода пыхи в саблайме.

AlertRED commented 5 years ago

при запросе POST /api/v0/users/group

"message": "Class 'App\\Http\\Controllers\\v0\\Validator' not found",

идеальный вариант это выносить валидацию в реквесты https://laravel.com/docs/master/validation#form-request-validation

Мне не нравилось что к каждому методу нужно создавать файл в котором не существенная проверка (проверка на валидность). Но я все таки смирился)

mx2s commented 5 years ago

просто сомневался, потому что ожидал, что об этом будет явно сказано в задании.

сомневаешься - спрашивай)

mx2s commented 5 years ago

Мне не нравилось что к каждому методу нужно создавать файл в котором не существенная проверка (проверка на валидность)

мне на самом деле тоже, но когда проект будет расти это будет все таки удобнее