[go: up one dir, main page]

Skip to content
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

Улучшение класса Keyboard #595

Merged
merged 14 commits into from
Nov 25, 2022
Merged

Conversation

wtlgo
Copy link
Contributor
@wtlgo wtlgo commented Nov 25, 2022

Какую проблему решает ваш PR:

Слегка изменил внутреннее устройство класса Keyboard. На мысль натолкнул issue #594. В принципе, в текущей версии VK API нет такой ситуации, когда пользователь хотел бы передать пустую строку в клавиатуру, но текущая имплементация все равно позволяет это делать.

Данное изменение добавляет возможность вызывать keyboard.row() сколько угодно раз подряд, а так же в самом конце, без вреда для здоровья создания лишних пустых строк.

Связанные issue: #594

  • Ваш код документирован.
  • Для вашего кода есть тесты.
  • Для вашего кода есть примеры использования в examples.

@FeeeeK
Copy link
Contributor
FeeeeK commented Nov 25, 2022

Для чего нужен флаг self.expect_new_line, почему бы просто не проверять наличие кнопок?

@FeeeeK FeeeeK changed the base branch from master to dev November 25, 2022 12:28
@wtlgo
Copy link
Contributor Author
wtlgo commented Nov 25, 2022

Нет, ну можно было бы конечно добавлять пустой список в конец, и чтобы Keyboard.row добавлял или не добавлял пустой список в зависимости от того, есть ли в конце уже пустая строка, и чтобы в методе Keyboard.get_json лишняя пустая строка в конце фильтровалась, однако это бы повлекло за собой лишний оверхед по памяти и вычислениям, а так же привело бы к тому, что такой код стало бы труднее поддерживать в будущем. Я уверен, что из всех доступных вариантов, вариант с флагом наиболее элегантно решает задачу.

@FeeeeK
Copy link
Contributor
FeeeeK commented Nov 25, 2022

Я считаю, что это должен решать юзер, а не фреймворк.

@wtlgo
Copy link
Contributor Author
wtlgo commented Nov 25, 2022

Опять таки, в каком таком сценарии юзер захочет передать в API пустую строку и получить невнятную ошибку в ответ, которая больше заблуждает, чем объясняет, что пошло не так?
На мой взгляд, если фреймворк позволяет лишний раз не выстрелить себе в ногу, это только идет в плюс. В особенности, что в худшем случае фреймворк от этого ничего не теряет, кроме возможности генерировать невалидную структуру, а в лучшем - улучшает юзер-экспериенс, позволяя писать более короткий и прямолинейный код.

@FeeeeK FeeeeK merged commit 58c5dfa into vkbottle:dev Nov 25, 2022
@Der4ik0
Copy link
Der4ik0 commented Nov 26, 2022

То есть что я вчера спрашивал, работает как я сделал вначале?

@wtlgo wtlgo deleted the enhance-keyboard branch November 27, 2022 00:54
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.

3 participants