-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ревью #2
Comments
Все типы с именем Json в директориях Telegram и Vk это объекты, определяемые конкретным сервисом. Они предназначены для использования с квалифицированным импортом, например:
Не уверен что стоит написать, "потому что это хендлер", "потому что точно будет посчитано"? Такую практику подсмотрел в этой статье, но почему именно там используются бенги не написано
Прям везде-везде для каждой функции? Не много ли визуального шума получится? И как же type inference?
Сделал часть логики через
Не совсем понятно что имеется ввиду, большая часть кода живет в IO и его тестировать в отдельности (аля юнит тест) не представляется возможным (да еще и зависимость от внешних ресурсов), а если вместе то это уже integration тест получится. Фримонадную логику самого бота (взаимодействие с пользователем через чат) я вроде протестировал с помощью тестового интерпретатора. Еще можно, наверно, на каждый парсер по паре тестов сделать, но это мне кажется излишним (или нет?). Для совсем простых функций тестирующая функция будет реимплементировать тестируемую функцию, что, как мне кажется, мало имеет смысла Остальные замечания постарался учесть и применить |
Окей, в таком виде это уже звучит логично, оставь так
Если ты не понимаешь зачем использовать фичу, то лучше её не использовать. Либо объясни, что без неё пойдет не так
Прям везде-везде для каждой функций (исключением может быть блок where, внутри него как правило сигнатуры не пишут), с сигнатурами твой код читать намного приятнее и проще.
Окей, как это работает, у нас есть какая-нибудь функция, которая работает с IO и ищет что-нибудь, к примеру
в коде, где она используется ты проверяешь её результат
Чтобы протестировать работу этой функции ты пишешь заглушку для неё, когда она работает корректно:
И вариант когда функция должна упасть упасть
Далее ты передаешь каждый такой handle в функцию которая его использует, и проверяешь, что эта функция должна вернуть в корректном варианте работы, а что в плохом. |
И пока все ещё есть вопросы к структуре проекта, ты понял, что ожидается увидеть, может есть вопросы? |
Сделал подобные тесты для
Да вроде все понятно, просто никогда не заморачивался и имел во всех проектах максимально плоскую структуру из модулей. Остальные замечания постарался учесть и применить |
Есть хорошее правило понятного кода: понтяному коду не нужны комментарии, в нем и так понятно что просходит :)
Я разберу на примере:
https://github.com/wixe/vktgbot.hs/blob/master/src/Telegram/User.hs#L12 - в этом случае наличие комментария точно избыточно, по названию понятно о чем идет речь. Если ты хочешь дать понять, что речь может идти о работе с ботом и юзером, ты мог бы назвать сам тип не Json, а, к примеру, UserBotInfo, эта идея относится ко всем типам с именем Json.
А вот здесь https://github.com/wixe/vktgbot.hs/blob/master/src/Bot.hs#L55 напротив стоило бы пояснить зачем ты переходишь от ленивых вычислений к активным, и тут бы действительно не помешал комментарий.
Ещё добавлю, что комментарии лучше писать либо сверху над чем-то, либо, в случае с полем, на определенном удаление:
Тогда это становится проще читать.
Модульность и стуктура проекта.
Есть несколько файлов, к примеру, вот этот https://github.com/wixe/vktgbot.hs/blob/master/src/Bot.hs сейчас его сложно читать, там идут в перемешку instance, функции и типы. Не бойся делать много папок и файлом, напротив, это хорошо и улучшает читабельность
В идеале каждый тип или группа связанных типов должны быть в своей папке и в своем файле.
Все функции которые работают с этим типом должны тоже должны лежать вместе, но не обязательно в том же файле, что и тип, тут уже смотри по ситуации.
Названия модулей и папок должны давать понять о чем идет речь и что лежит внутри модуля/папки
Функции и типы
Если идет несколько вложенных case of или довольно большое тело функции, по возможности старайся разбивать это на смысловые части, выносить что-то в блок where, использовать let, выносить функцию наружу.
К примеру, такая лямбда выглядит очень сложной: https://github.com/wixe/vktgbot.hs/blob/master/src/Bot.hs#L194
Не забывай везде писать сигнатуры: https://github.com/wixe/vktgbot.hs/blob/master/src/Options.hs#L45
Тесты и ошибки.
Здесь все те же рекомендации, что и выше, но добавлю, что тестов явно должно быть больше, чтобы они по максимум покрывали всю логику.
Очень советую использовать Handle Pattern:
https://jaspervdj.be/posts/2018-03-08-handle-pattern.html
https://www.schoolofhaskell.com/user/meiersi/the-service-pattern
Он поможет тебе организовать логику и организовать тесты.
Я не особо увидел, чтобы ты где-то ловил ошибки и где-то их бросал, вот что советую почитать https://hackage.haskell.org/package/base-4.14.0.0/docs/Control-Exception.html
Постарайся ловить ошибки везде где это возможно, а после, написать тесты для каждой такой ошибки, где рассматривается случай что ошибка происходит и случай что все работает корректно
The text was updated successfully, but these errors were encountered: