AKitavtsev / telegram-vk-bot

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

Наименования типов и методов #17

Closed kelizarov closed 3 years ago

kelizarov commented 3 years ago

Рекомендую при объявлений типов использовать ADT по максимум чтобы отразить предметную область в типах.

Например есть словарь который называется MapInt. Другим разработчикам будет не так просто понять назначение такой абстракции, пока не посмотрят на то как этот тип используется в коде. Желательно проверить это на всех типах данных, которые используются в проекте.

Другое замечание по поводу нейминга методов и переменных внутри функции: желательно стараться полностью отразить их назначения в названиях, так чтобы было сразу понятно что на в данной строчке коде происходит. И желательно не злоупотреблять сокращениями, так как они увеличивают когнитивную нагрузку при чтении кода.

И еще одно предложения: можешь пожалуйста оформлять изменения в виде мерж реквестов? Это поможет нам лучше оценить изменения и комментировать код, чтобы быстрее дать фидбек :)

AKitavtsev commented 3 years ago

ADT, мерж реквест -что-то знакомое, но ... Можно, чуть подробнее, или ссылку. Согласен, фантазии для нейминга не хватает. Ведь еще надо не затенять уже существующий контекст. Буду стараться. А уж в совсем локальных функциях (когда все на одной - двух строках), то же надо об этом думать? Например, к этому коду есть замечания?

newDict :: Upd a => DataLoop a -> DataLoop a newDict dl = dl {dictionary = dict} where dict = updateDictionary (dictionary dl) $ getUserAndNumRep (updates dl) updateDictionary d [] = d updateDictionary d ((k, v):xs) = updateDictionary (M.insert k v d) xs

kelizarov commented 3 years ago

По поводу мерж рекветса: я предлагаю новые изменния делать в отдельной ветке от мастера (или мейна), а затем открывать Pull Request с этими изменениями, при этом указав ссылку на ишью. При этом не делать все изменения в одной ветки сразу, а делать их изолировано друг от друга.

Про АДТ я имел ввиду максимально задейстовать собственные типы данных, чтобы лучше представить предметную область проекта на типах. В коде есть пару мест, где тип можно заменить со String на сумму типа, чтобы лучше представить перечисляемые возможности значения.

По поводу примера: тут еще можно исправить некоторые момента. Например, dict и dictionary могут читаться как одна и та же сущность, хотя у каждого есть свое предназначение в данной функции. Можно исправить это добавив нужный префикс переменной, чтобы нагляднее показать что она за собой прячет. Простое сокращение наоборот начинает запутывать пользователя, и приходятся чуть больше времени тратить на чтение кода.

AKitavtsev commented 3 years ago

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

  1. Вот как организовать ветку на практике? Вот есть каталог проекта. Я вношу изменения в определенные файлы. Add, Commit. Push в другую ветку. Теперь мне надо вносить другие изменения в проект. В какой проект? Тот же уже исправлен? А старый, что надо клонировать в другой каталог? И там вносить изменения? И снова в новую ветку? А если я исправлял одно и тоже место? Как слить ветки с двумя разными исправлениями. Конечно, документация на Git есть, но нет примеров. Очень страшно натворить делов. Я думал, что по разным веткам разные разработчики с разными модулями, или, хотя бы, функциями. А тут сам с собой по разным веткам.
  2. Очень странно про пару мест со String и суммой. Если, это так, это какой-то детский сад с моей стороны (не исключаю). А, можно пример?
  3. Вот я привел пример с диктами (как у меня), а можно ответный пример (а как надо).? А то я неправильно пойму и на исправляю...
AKitavtsev commented 3 years ago

MapInt - исправил больше ничего не вижу. String вместо cумм тоже не вижу С мерж реквестами разбираюсь.

kelizarov commented 3 years ago

MapInt - исправил больше ничего не вижу. String вместо cумм тоже не вижу С мерж реквестами разбираюсь.

Могу пример привести: у тебя в конфигах есть поле сonfigApi которое определяет какого бота нужно запустить. Сейчас оно String, но другому программисту ничего не мешает вписать любое значение, и оно по системе типов будет валидным, но при запуске кода получится так что он будет запускать телеграм бота. Вот это будет не очевидное поведение системы, которое можно было закрыть тем, чтобы на типах указать какие варинты значений может быть, применив ти суммы.

AKitavtsev commented 3 years ago

Спасибо.

Исправлено