Skip to content

Refactoring and optimization #75

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

dakalamin
Copy link

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

Пару комментариев:

059b061 - Remove redundant conditions
Во всех этих ситуациях значения можно сбрасывать без их предварительной проверки - результат будет один и тот же

13c4bf7 - Optimize and simplify
MultiButton.h - bool tick(T0& b0, T1& b1) дублирует код из bool VirtButton::tick(VirtButton& b0, VirtButton& b1), не хватает только b0.call() и b1.call()
VirtEncoder.h - Аналогично с int8_t tickRaw(const bool e0, const bool e1) и int8_t tickISR(const bool e0, const bool e1)
VirtEncButton.h - В bool _tickRaw(bool btn, int8_t estate) можно обойтись без флага encf

Остальное:

  • В MultiButton.h шаблонные T0 и T1 никак не подсказывают, объектами каких классов они могут быть. Тут же проблемы отсутстувия подсветки синтаксиса, эльфийские сообщения комплиятора, если есть ошибки с этим классом, и запутанная ситуация с практической разницей между MultiButton::tick(T0& b0, T1& b1) и VirtButton::tick(VirtButton& b0, VirtButton& b1). Как пример того, что проблема существенная, это ошибка в README:
    2. Передать виртуальной кнопке в обработку свои кнопки (это могут быть объекты классов `VirtButton`, `Button`, `EncButton` + их `T`-версии). **Мульти-кнопка сама опросит обе кнопки!**

    Попытка передать VirtButton в MultiButton приводит к ошибке, т.к. у VirtButton нет метода tickRaw(), только tickRaw(const bool s):
    #include <EncButton.h>
    
    VirtButton a, b;
    MultiButton c;
    
    void setup() { }
    void loop() {
      c.tick(a, b);
    }
    Из потенциальных решений - завести интерфейс для кнопки по типу:
    class IButton {
    public:
      virtual bool read() = 0;
      virtual bool tick() = 0;
      virtual bool tickRaw() = 0;
    };
    и наследовать его в классах Button/ButtonT и EncButton/EncButtonT (в таком случае, возможно, стоило бы сделать то же самое и с классом энкодера)
  • В README несколько раз упомянается метод readEnc для EncButton/EncButtonT, которого не существует:

    EncButton/README.md

    Lines 228 to 230 in 489c34a

    | | VirtEncoder | Encoder | VirtEncButton | EncButton |
    | -------------- | :---------: | :-----: | :-----------: | :-------: |
    | readEnc | | | | ✔ |

    EncButton/README.md

    Lines 615 to 616 in 489c34a

    // прочитать значение энкодера
    int8_t readEnc();

    Cоответственно, следует либо добавить его в код, либо убрать его из README
  • Английская версия README - это полная катастрофа, где все от бессмысленного текста до сломанного форматирования. Тут либо выбрать другой сервис для автоматического перевода и после вручную проверять Markdown форматирование, периодически синхронизируя с обновлениями в основном файле, либо совсем отказаться от переведенной версии, оставляя это на потенциального пользователя

@GyverLibs
Copy link
Owner

GyverLibs commented Aug 1, 2025

очень не со всем согласен, что то выглядит как потенциальный баг, перетащу частично вручную. Спасибо

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants