Где получить code review?

Question

[Александр Романов]

У меня есть код, который мне некому показать и хотелось бы получить по нему какие-нибудь вменяемые комментарии. Где это можно сделать?
Вот код: https://github.com/yetione/mdCms :)

Answer 1

[profesor08]

Приведи бардак в папке www в порядок.

Comments

[Александр Романов]

Там, в принципе, порядок

[Lander]

Александр Романов, Думаю что имелось ввиду то, что такие вещи как
Dompdf
GuzzleHttp
Kladr
PHPMailer
и так далее - являются модулями. Но почему то за папку Modules они вынесены.

[Александр Романов]

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

[Eugene Mosyukov]

Александр Романов, внешние библиотеки обязательно нужно делать зависимостями композера, а не включать в свой код.

Answer 2

[Boris Köln]

.bak не должен быть в репозитории

import_csv.php - говнокод.
1. Пароли надо хранить в конфиге вне репы.
2. А если кто-нибудь укажет example.com/import_csv.php ? БД засрется

Все файлы cms надо хранить вне www.

Например, Modules/Food/Food.php
1. мало phpDoc
2. сохранять объекты в сессию - потенциальные проблемы
3. зачем $cart->setCity в saveCart()? Переменная $cart будет удалена при выходе из метода.
4. recountOrder не нужен. Вместо него - триггеры в БД
5. fixOrders не нужен. Вместо него - внешние ключи

В остальном вроде неплохо.

Comments

[Александр Романов]

2 и 3 пункты связаны. Как раз что бы в сессии не хранить объекты с ссылками на соединение с БД. Перед этой строчкой есть строчка $cart->unsetCity();

[Александр Романов]

а import_csv и правда гвонокод, он там вообще не нужен:)

[Boris Köln]

Допустим, в сессию сохранили без city. Зачем далее setCity? Это бессмысленно, ибо далее нет return $card;
Какой смысл менять переменную и затем сразу же ее удалять?

[Александр Романов]

Boris Korobkov, Cart не удаляется, т.к. это объект, который продолжает существовать и далее по коду. Сейчас уже точно не скажу где, но это дальше где то нужно.

[Boris Köln]

Область его видимости ограничена функцией, то есть он удаляется на следующей строчке, где функция заканчивается

[Александр Романов]

Boris Korobkov так в переменную $cart записывается $this->cart при вызове getCart, т.ч. это ссылка на свойство объекта. Или я что то не понимаю?

[Boris Köln]

Тогда ok.
Хотя лучше писать код так, чтобы избегать неочевидностей.

Answer 3

[nepster-web]

От себя добавлю

- код не по PSR
- на дворе 2017 заканчивается вообще-то

Response::FORMAT_JSON=>array(
            'className'=>'\\Core\\Response\\JSONResponse',
        ),

- переведите комментарии на англ.
- перестаньте поддерживать php
- прочитайте про SOLID
- разграничте статику и серверные скрипты

Comments

[Александр Романов]

Какие варианты решения 2 пункта? фабрика через ифы? У меня приходит в GET format=json.

[nepster-web]

Я скорее имел ввиду синтаксический сахар современных подходов:

Response::FORMAT_JSON => [
    'className' => \Core\Response\JSONResponse::class,
],