«phpClub» — архив тем ("тредов"), посвящённых изучению PHP и веб-технологий.
Аноним 2018/09/24 21:55:37  №1269777 1
Ответы: >>1269785 >>1270395
Аноним 2018/09/24 22:04:00  №1269785 2
>>1269777
По какому гайду учишься?
Ответы: >>1269789
Аноним 2018/09/24 22:17:02  №1269789 3
>>1269785
По гуглу и официальным документациям. Что интересно - в то и ныряю. К сожалению, за ручку как ОП никто не водит.
Ответы: >>1269792
Аноним 2018/09/24 22:25:46  №1269792 4
>>1269789
Давно занимаешься?
Ответы: >>1269797
Аноним 2018/09/24 22:32:09  №1269797 5
>>1269792
Урывками оооочень давно, иногда забивается на по полгода. Это интересное хобби. Жаль разделить не с кем.
Ответы: >>1269799 >>1269950
Аноним 2018/09/24 22:36:16  №1269799 6
>>1269797
>Жаль разделить не с кем
Можно попробовать
Ответы: >>1269800 >>1269801 >>1269950
Аноним 2018/09/24 22:41:42  №1269800 7
Аноним 2018/09/24 23:04:21  №1269801 8
Аноним 2018/09/25 10:20:43  №1269950 9
>>1269799
>>1269797
Пробовал, жалко не получалось. Можем втроека
Ответы: >>1269957
Аноним 2018/09/25 10:47:13  №1269957 10
>>1269950
Ну мне трудно представить, как это все выглядит.
Аноним 2018/09/26 01:36:49  №1270395 11
>>1269777

> https://github.com/wheelafterwheel/TDG/blob/master/sqlite3/transaction.ts

Здесь проблемы с читаемостью. Мне кажется, надо было не создавать объект to внутри функции, а сделать отдельным классом. Так как получилась тяжело читаемая функция-монстр.

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

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

В строке 47 глубина отступов составляет 7 - по моему, это перебор.

Возможно, стоило сделать промисифицированную обертку над db отдельным классом - так как многочисленные if ... reject только ухудшают читаемость. То есть сделать PromisifiedDb и уже с ним тут работать:

await pdb.beginTransaction();
to = new DbExecutor(pdb);
await sequence(to);
await pdb.commit();

Может быть, даже DbExecutor и не нужен будет, можно будет передавать в sequence этот PromisifiedDb.

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

Также, я не думаю, что надо было в классе делать db.open/close(). Логичнее передавать уже открытый объект db. Как я понимаю, при коммите изменения гарантированно сохраняются и без close(). Заодно мы могли бы поставить тайп-хинт в конструкторе TDG, а не принимать тип any.

Также, я тут вижу потенциальную ошибку из-за асинхронности. Ты ждешь (await) завершения функции sequence и закрываешь транзакцию. Но кто гарантирует, что в этой функции не будут делаться вызовы объекта to после закрытия транзакции? Это вполне возможно из-за асинхронности:

transaction(function (to) {
setTimeout(function () {
to.execute(...);
}, 1000);

await to.execute(...)
});

Можно защититься от этого добавлением флага в to, который показывает доступность объекта, включается после выполнения BEGIN и выключается перед COMMIT.

То, что rollback реджектится не очень логично - наверно, все же логично при успехе отката резолвить его в null или true.

Также, транзакции конечно нужны при записи в БД, но при чтении они выглядят немного избыточными.

> if (
> statement.match(/^\s*SELECT/)
Это плохая идея, есть например SHOW и может еще какие-то команды. Лучше не определять их автоматически.

> async slice(limit: number = 30, offset: number = 0):
Это довольно ограниченный метод, он не поддерживает ни условия, ни порядок сортировки.

Не уверен, что нужны методы create/drop. Обычно таблицы создаются и изменяются через миграции.

> export class Staff extends TDG {
> constructor(
> name: string = 'staff'
Непонятно, зачем нужна возможность задать другое имя.

> (\`password_hash\` = ? OR \`password_hash\` IS NULL)
Это немного странно, а не приведет ли это к авторизации с пустым паролем?

> return (result["0"]["0"] !== undefined)
Надо было сделать методы, возвращающие одну строку и одну колонку.

Я вижу, что у тебя есть тест для Staff, а что насчет тестов для остального кода?

В тесте для ожидания исключения есть вроде бы конструкции expect().to.throw(...), а у тебя, если исключение не произойдет, то ошибки не будет.

Тест не очень хороший. У тебя просто один гигантский сценарий. Но тесты (по моему) должны тестировать фичи:

- вставка: если вставить объект в таблицу с помощью TDG, то его можно в ней найти
- удаление: если удалить объект с помощью TDG, то он больше не находится
- поиск: функция поиска находит объекты по правильным критериям (и не находит по неправильным)
- итд.

То есть ты должен подумать, что умеет делать твой TDG, записать это в виде списка фич, требований, и на каждое написать тест. Причем в идеале отдельный тест пишется для базового класса и отдельный для Staff.

Это дает такие преимущества:

- отдельные тесты получаются меньше и проще, и тестируют ровно 1 фичу
- если что-то сломается, то видно будет, какие фичи сломаны

И мне кажется, сама архитектура mocha к этому подталкивает: там есть describe() для описания тестируемого объекта и it() для описания фич. Ты же используешь it() для описания шагов гигантского теста. А должно быть:

describe('Staff', function () {
it('Can insert employee', ...);
it('Can delete employee', ...);
....
});

Если что, я могу еще что-то подсказать по тестам. Было бы хорошо тогда покрыть тестами весь код.

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

Главная проблема - нечитаемость кода в transaction.ts.

Если хочешь поломать голову, не хочешь ли попробовать сделать ORM на основе DataMapper, вроде Доктрины в PHP? То есть сделать так, чтобы не надо было вручную писать методы вроде register(), а они бы как-то сами умели определять структуру объекта и формировать SQL код:

var repo = Repository.getFor(Employee);
var e1 = repo.findOneBy({'tableNumber': 123});
e1.name = 'New name';
repo.flush(); // сохраняет изменения в БД

Далее, реализовать паттерны Identity Map, Unit Of Work. Далее, связи между сущностями и ленивую загрузку. Задача интересная, хоть и сложная.
Ответы: >>1270593
Аноним 2018/09/26 15:08:57  №1270593 12
>>1270395
Слава слонику, ты тут многие замечания описал, которые мне тоже не нравятся (ура!). Ремарочка, как появился transaction.ts: я замучался с дефолтной реализацией драйвера sqlite3, и решил обыграть все спорные моменты одной функцией (написал и забыл, работает же!). Однако этот франкенштейн не так гибок как хотелось бы (появились ненужные флаги и прочий мусор), ты прав, лучше переписать ее или вообще убрать. (Но повторюсь, от дефолтного апи меня аж трисет, вот оно https://github.com/mapbox/node-sqlite3/wiki/API , если подскажешь, как это превратить в promisified, буду очень благодарен). Вообще у меня идея появилась сейчас абстрагироваться от баз данных вообще ииии да, именно
>сделать так, чтобы не надо было вручную писать методы вроде register(), а они бы как-то сами умели определять структуру объекта и формировать SQL код
На вскидку это довольно сложно, но я попробую, обучение же! (Но в реальном проекте лучше не велосипедить и взять https://github.com/typeorm/typeorm)

По поводу отступов:
>отступы в 4 пробела ухудшают мне читаемость, может конечно я просто к ним не привык, но они же слишком длинные
ну ты понял, алсо https://github.com/basarat/typescript-book/blob/master/docs/styleguide/styleguide.md#spaces
Я думаю, два, четыре пробела или таб не делают программиста.

>глубина отступов составляет 7 - по моему, это перебор
ну да, чет многовато (даже для такой трешовой функции)

Главное:

>Как я понимаю, при коммите изменения гарантированно сохраняются и без close()
из документации вообще не понятно, нужно ли, и когда нужно закрывать подключение (может быть принудительное закрывание помогает избежать утечек памяти или предотвращать коррупцию файла, хз), поэтому я на всякий случай закрываю. Однозначно нужно избавляться от всех any потому что это херня а не тип.

>То, что rollback реджектится не очень логично - наверно, все же логично при успехе отката резолвить его в null или true.
ты имеешь в виду to.rollback() ? это мануальный откат, который мы сами вызовем при определенных условиях в своем коде, указывая причину, которую (та-даа!) можно отловить (потому-что даже мануальный откат это исключительная ситуация). При reject мы выходим из sequence и идем ловить ошибку, а вот при resolve(true) мы как бы уже откатились, но команды далее по коду будут как ниндзи выполняться на уже закрытую транзакцию, как будто все нормально. (Собственно, это проблема архитектуры функции)

>вызовы объекта to после закрытия транзакции? Это вполне возможно из-за асинхронности...
жесть конечно, это надо быть кем, чтобы так стрелять себе в ногу (мы делаем из кучи асинхронных функций одну "синхронную", чтобы потом в коде ее запускать по таймауту (хм...асинхронно?))
Тогда да, флаг тут - очень хорошее решение, спасибо.


Я думал ты будешь пробовать сам запускать, ну да ладно.

Остальные замечания: все постараюсь исправить. Нужны комментарии на вопросы выше.