pf03 / newBot

BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Замечания по коду #17

Closed KateBushueva closed 3 years ago

KateBushueva commented 3 years ago

модуль Common.Functions

template :: String -> [String] -> String
template str args = foldl f str $ zip ts args
  where
    ts = map (\n -> ('{' : show n) ++ "}") [0 :: Int, 1 ..]
    f :: String -> (String, String) -> String
    f acc (t, arg) = replace t arg acc
    replace :: String -> String -> String -> String
    replace t0 s str0 =
      let strs = splitOn t0 str0
       in concat $ init $ foldr (\part acc0 -> part : s : acc0) [] strs
KateBushueva commented 3 years ago

модуль Transformer.Internal исключения в IO монаде бросаются через throwIO

KateBushueva commented 3 years ago

модуль Logic.App комментарии, которые дублируют код, не нужны в остальном по читабельности этот модуль мне нравится

KateBushueva commented 3 years ago

модуль Logic.Logic

KateBushueva commented 3 years ago

модуль Messenger.Bot.Class я бы сделала проще, без TypeFamilies. Но не против оставить как есть. Здесь необходимо убрать закомменченный код

pf03 commented 3 years ago

модуль Common.Functions

template :: String -> [String] -> String
  • непонятные названия переменных, можно увеличить читабельность функции введя понятные наименования
  • использование небезопасных функций (init) без проверки

исправил

checkUnique :: Eq a => [a] -> Bool
checkUnique l = let newList = filter (== True) ((==) <$> l <*> l) in length newList == length l

очень нерациональная проверка на уникальность

Исправил через сортировку, сложность n*log n вместо n^2. Там список из двух имен ботов проверяется, это вообще не критично

pf03 commented 3 years ago

модуль Transformer.Internal исключения в IO монаде бросаются через throwIO

исправил. Ошибки в чистом коде бросаются в Either. Ошибки в монаде IO - throwIO Ошибки в монаде Transformer - liftIO . throwIO

pf03 commented 3 years ago

модуль Logic.App комментарии, которые дублируют код, не нужны в остальном по читабельности этот модуль мне нравится

исправил

pf03 commented 3 years ago

модуль Logic.Logic

  • убрать закомменченный импорт
  • в функции toMessageCommand постараться избавиться от вложенный кейсов

исправил, хотя 2 уровня вложенности допускается, как указано в риззоме

pf03 commented 3 years ago

модуль Messenger.Bot.Class я бы сделала проще, без TypeFamilies. Но не против оставить как есть.

Я сделал синтаксис по образцу Алексея

Здесь необходимо убрать закомменченный код

исправил

KateBushueva commented 3 years ago

модуль Interface.Cache.Config.State

KateBushueva commented 3 years ago

модуль Interface.Error.Functions

KateBushueva commented 3 years ago

модуль Interface.Log.Class ?

KateBushueva commented 3 years ago

модуль Interface.Log.Functions

KateBushueva commented 3 years ago

модуль Interface.Log.Types не нужно оставлять комментарии, которые дублируют код

KateBushueva commented 3 years ago

модуль Interface.MTrans ?

KateBushueva commented 3 years ago

модуль Logic.VK.Query.Functions функция sendMessageQuery : странно с точки зрения восприятия использование Either монады, где у тебя левый конструктор приводит к корректному выполнению функции, а правый конструктор приводит к ошибке.

KateBushueva commented 3 years ago

модуль Messenger.Bot.Telegram.Instances

аналогично в модуле Messenger.Bot.VK.Instances

KateBushueva commented 3 years ago

модули Messenger.Bot.VK.Types и Messenger.Bot.Telegram.Types убрать ненужные расширения

KateBushueva commented 3 years ago

модуль Common.Types

KateBushueva commented 3 years ago

модули Messenger.Update.Telegram.Types и Messenger.Update.VK.Types

KateBushueva commented 3 years ago

Кстати, по поводу расширений: ты можешь прописать необходимые в package.yaml файле (как это у тебя сделано с OverloadedStrings). В таком случае не будет необходимости прописывать их в каждом файле, где требуется

KateBushueva commented 3 years ago

модуль Parse.Telegram.Functions

KateBushueva commented 3 years ago

модуль Parse.Telegram.Internal

KateBushueva commented 3 years ago

Все, с моей стороны больше претензий к боту нет

pf03 commented 3 years ago

модуль Logic.VK.Query.Functions функция sendMessageQuery : странно с точки зрения восприятия использование Either монады, где у тебя левый конструктор приводит к корректному выполнению функции, а правый конструктор приводит к ошибке.

в переменной eMessageCommand монада Either используется в контексте "сообщение или команда" а не для обработки ошибок. Такая структура Update.Update прописана в Messenger.Update.VK.Types

type Update = (ChatId, Entity) data Entity = Entity {body :: Either Message Command, attachments :: [Attachment]} deriving (Show)

А уже в выходной переменной идет контекст обработки ошибок.

KateBushueva commented 3 years ago

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

pf03 commented 3 years ago

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

исправил на сумму типов

pf03 commented 3 years ago

исправил по всем пунктам