«phpClub» — архив тем ("тредов"), посвящённых изучению PHP и веб-технологий.
Аноним 2020/07/22 14:24:05  №1757803 1
ОП, глянь, пожалуйста, задачу про файлообменник

https://github.com/deadj/uppu

Из твоих прошлых замечаний не исправил:
>Здесь вместо массивов можно было бы попробовать сделать классы, для разных видов файлов, вроде VideoParams, AudioParams итд.
Мне всё же кажется, что массивы в данном случае лучше
>Вообще, для преобразования объекта в массив (и обратно) есть библиотеки вроде JSMSerializer. Может, она тебе тут пригодилась бы?
Кода для пребразования мало, и подумал, что пусть будет
>Используй lastInsertId() вместо
>foreach ($filesId as $id) {
> $filesArray[] = $this->filesTable->getFileThroughId(intval($id));
Потому что во время операции может быть добавлен другой файл, и id возьмётся не тот

Заранее спасибо тебе, ОП
Ответы: >>1781062
https://github.com/deadj/uppu Аноним 2020/08/18 22:07:19  №1781062 2
>>1757803

composer.lock принято коммитить, чтобы у всех разработчиков были одинаковые, проверенные версии зависимостей. В БД стоит прописать внешние ключи для связей между таблицами (для полей comments.parentId, comments.fileId).Немного странно, что comments.fileId имеет тип VARCHAR, а не INT (не ссылается на id файла). Обычно в таких случаях ставят ссылку именно на id, так как он меньше и он является первичным ключом таблицы.

> parse_ini_file('init.ini');

Лучше использовать тут __DIR__, иначе файл ищется в текущей директории процесса PHP (а не рядом с dbConfig.php), и кто знает, чему она может быть равна.

Для функции deleteErrorFiles, лучше было сделать скрипт, который запускается в командной строке, а не доступное через веб действие.

> $this->filesTable = new FilesTable($db);

Класс FilesTable стоило сделать сервисом, то есть засунуть в контейнер Слима, и получать из него. А то в коде часть классов в контейнере, а часть нет. Как-то нелогично. GearmanClient тоже в контейнер стоило засунуть.

> return $this->view->render($response, '404.html');

При отдаче страницы 404 надо отдавать еще код HTTP статуса 404, чтобы роботы тоже понимали, что такой страницы нет.

> $dataForView = array(
>\t\t\t'filesData' => array(
>\t\t\t\t'nameId' => $file->getNameId(),
>\t\t\t\t'name' => $file->getName(),
>\t\t\t\t'link' => $file->getLink(),

Тут проще передавать во view объект $file и в шаблоне уже вызывать его методы.

Одинаковые части в шаблонах (например, шапка) стоит реализовать с помощью наследования от базового шаблона (еще его называют layout). В Twig есть такая возможность.

> <img id="imgFile" src="/{{ filesData.link }}" width="350">
Тут лучше использовать max-width, а то маленькие картинки будут увеличены. Ну и по-хорошему, стоит генерировать уменьшенные превьюшки для экономии трафика.

> <ul class="dataNames">
> <ul class="dataValues">
Тут не логичнее ли было применить тег table и сверстать как таблицу?

Для процесса загрузки файла можно было сделать отдельный сервис, тогда мы бы могли вызывать функцию сохранения файла откуда угодно. А так, ты захочешь сделать API для загрузки файлов или CLI скрипт, и придется выносить код из контроллера MainController в сервис.

> $uploadIsDone = "null";
Это стоило сделать константой вроде File::STATUS_DONE. Типы файлов тоже сделать константами.

> \t $nameId = preg_replace('~files\\/\\d{2}_\\d{2}_\\d{2}\\/~ui',"", $link);
> \t $nameId = preg_replace('/[.]\\S*/', "", $nameId);
Эти преобразования путей стоило вынести в функцию, а не размазывать по коду. Есть же функция createFilesLink, можно сделать и обратную.

> private function convertArrayIdToString
Тут не проще было использовать встроенную функцию implode() вместо цикла?

> public function getFilesArrayThroughId(string $stringId): array
Тут было бы правильнее передавать массив id и уже внутри FilesTable конвертировать его в строку. Это ответственность TableDataGateway, знать синтаксис SQL и формировать список, а не отвественность внешнего кода.

Получать id вставленного файла стоит через lastInsertId, он вернет верный id, даже если в это время второй процесс успел добавить еще один файл в таблицу. А твой код - не защищен от этой ошибки. Либо, как вариант, генерировать уникальный id самому до вставки в БД.

Так в общем, для начинающего неплохо сделано.
Ответы: >>1785878
Аноним 2020/08/23 18:19:02  №1785878 3
Если вас пропустили в предыдущем треде, можете напомнить о себе тут. Также, напомню, что код файлообменника https://github.com/deadj/uppu я проверил тут: >>1781062 →