Оценка кода

[fixxxer]

Джун, если про 7 лет общего и 3 года опыта Symfony правда, то надо внимательно смотреть в чем в принципе проблема, что за такое большое время код остался на таком уровне.

Ну может варился в собственном соку, или среди 19-летних "сеньоров"

при хорошем наставнике и правильном восприятии критики

Ну вот на восприятие критики и надо сразу проверить.

 

[scorpion-ds]

А на

throw $this->createNotFoundException('You can not edit this project');

кто-нибудь обратил внимание?…

Не обратил, в принципе я бы добавил ID юзера в сам запрос, но если надо вывести сообщение о том, что проект есть, но нет доступа, то конечно не мешал бы другой экзепшен, думаю это невнимательность.

 

[Redjik]

жесть - до меня только сейчас дошло с эксепшеном...
@scorpion-ds, там вдругом прикол

 

[Adelf]

@Redjik, в одном месте забыт throw...
Ну и возврат 404 вместо 403 иногда, хотя это мелочи. Я в симфониях не эксперт. Что там?

 

[grigori]

HR говорит, что на должность "middle/senior" нехорошо давать тестовое задание, хотя я хотел.

Пусть расскажет это сотрудникам Toptal

 

[hell0w0rd]

А на

throw $this->createNotFoundException('You can not edit this project');

кто-нибудь обратил внимание?…

Вот для этого и надо по коду сразу с кандидатом смотреть.
Тут можно только гадать, он не знает, что в таких случаях выдают 403, или хотел сделать, как делает гитхаб - показать 404 страницу, если нет доступа, но зафейлил желание сообщением.

 

[scorpion-ds]

Товарищь @Redjik, говорит, что там в другом прикол ...

 

[Redjik]

да все просто
делают обычно

$this->throwNotFoundException($message);

если уж так лень создавать объект и код ошибки передавать
а тут - колхоз какой то...
нахрена ему обьект создавать, где этот метод будет использоваться в отрыве от throw (ну кроме ошибки в коде, как у автора)

 

[Adelf]

@Redjik, ты не поверишь
http://symfony.com/doc/current/book/controller.html#managing-errors-and-404-pages

 

[Redjik]

вот колхозники =)
я знаю прекрасно, что бывают случаю, когда такой подход оправдан, но ни разу не с NotFoundHttp

 

[Redjik]

Я даже не поленился, посмотрел, кто это закоммитил
https://github.com/weaverryan
вот оно - лицо колхозника

ЗЫ. pr
https://github.com/symfony/symfony/pull/394

 

[флоппик]

Ну я вот кстати, иногда лохуюсь и пишу вместо throw Exception — return Exception. Хорошо хоть лярва достаточно умная, что бы понять, что я хочу)

 

[Вурдалак]

жесть

вот колхозники =)

а тут - колхоз какой то...

Это ты из-за $this->createNotFoundException(), я ничего не упускаю?

 

[Adelf]

@Вурдалак, да. Он хочет $this->throwNotFoundException()

И интервьюруемый как раз ошибся из-за этого. в одном из 43 вызовов - забыт throw

 

[scorpion-ds]

@Redjik, ясно, но я тоже так пишу, взял как раз из этой доки такой подход.

@Вурдалак, да. Он хочет $this->throwNotFoundException()

В самой Symfony нет же "throwNotFoundException" ...

Вообще я некоторое время делал примерно так, для выхода в случае ошибки:

return new Response(null, 404);

 

[Adelf]

@scorpion-ds, он критикует симфони, а не твоего "сеньора"

 

[AnrDaemon]

IMO, если ты хочешь выбросить эксепшен, сделай это!
Не надо писать "throw $this>ХЗ(WTF)". Пиши то, что хочешь написать. "throw new ХЗException(WTF)"

 

[grigori]

а можно для тех, кто в танке, перечислить какие принципы SOLID, или другие какие-то, нарушаются в throw $this->createNotFoundException() ?

 

[AnrDaemon]

Глядя на код, совершенно неочевидно, какой эксепшен ожидать, раз. И лишняя строчка (хорошо если одна) в стеке - не проблема, но тем не менее.

 

[mz]

@AnrDaemon, а если однотипные конструкции, вроде throw new HttpException(404, "Not Found"), тоже будешь дублировать их? Методы-билдеры хорошая практика в .NET, колхозом это уже назвать нельзя.

Только щас заметил: что за хрень такая вот это https://github.com/symfony/symfony/tree/master/src/Symfony/Component/HttpKernel/Exception ? Почему не обошлись одним HttpException? А если уж взялись, то надо было идти до конца и создать исключения для всех ошибок в HTTP.