Skip to content

Centralize gson-calls, generics and simplified Keyboard creations #208

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

Closed
marcovaneck opened this issue Jul 29, 2020 · 2 comments · Fixed by #209 or #214
Closed

Centralize gson-calls, generics and simplified Keyboard creations #208

marcovaneck opened this issue Jul 29, 2020 · 2 comments · Fixed by #209 or #214

Comments

@marcovaneck
Copy link
Contributor

marcovaneck commented Jul 29, 2020

Hi,

I'm new to telegram and wanted to create a simple bot. Found this project, a nice and clean api. But noticed some improvements to the keyboard creation can be made. Since this is opensource, let's share my poposal.

Create keyboards like:

new ReplyKeyboardMarkup().addLine("Button0.1","Button0.2").addLine("Button1.1","Button1.2").oneTimeKeyboard(true).resizeKeyboard(true)
new ReplyKeyboardMarkup("Button0.1","Button0.2").resizeKeyboard(true)

See my fork: #https://github.com/marcovaneck/java-telegram-bot-api

Best regards,
Marco #

@pengrad
Copy link
Owner

pengrad commented Jul 29, 2020

Hi Marco!

This is a very nice general cleanup!
I didn't update many things after switching to java8.

But well, there are many changes, not sure I would like to merge all of them :)
Like I would not modify TelegramBotClient response handling, keep toWebhookResponse() method in request.
Anyway if you want to bring all these changes we need to split them by separate commits/PRs and we can discuss it 1 by 1.

New keyboard creations are super convenient, agree!
Why deprecate existing constructors?
I think they still work fine, no need to warn users to migrate.
I would also rename line to row, because this word is used in telegram doc Array of button rows.

A bit against of using java streams.
There was a case when this library was used in android app (surprise-surprise) and streams will not work there (actually can by requires some setup).
So if we can do without stream I would prefer it :)

One small comment - need to apply code formatting, mostly spacing around commas, parameters, colons.

If you agree with these comments, please feel free to open pull request!

Stas

@marcovaneck
Copy link
Contributor Author

#209
Hi Stas,

Thanks for the quick response! I applied you suggestions (hope I didn't missed any) and put them in a pull request. The request now contains the keyboard changes and the trivial generics fixes. The deprecate constructors are removed, I'm not a fan of arrays so want to inform the users of the new of creating keyboards. The change to the response-handling is also reverted. Only wrote them to resolve a progamming error in my code.

Marco

This was linked to pull requests Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants