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

iq_res_title should be required (not Maybe) #99

Open
fizruk opened this issue Jun 28, 2017 · 4 comments
Open

iq_res_title should be required (not Maybe) #99

fizruk opened this issue Jun 28, 2017 · 4 comments

Comments

@fizruk
Copy link
Contributor

fizruk commented Jun 28, 2017

Currently it's like this:

iq_res_title :: Maybe Text

See InlineQueryResultArticle documentation.

@klappvisor
Copy link
Owner

klappvisor commented Jun 28, 2017

@fizruk Unfortunatelly there is an optional title in the other constructors, so it's done this way as common denominator

@klappvisor
Copy link
Owner

@fizruk But if you can suggest better option, I would be happy to implement it

@fizruk
Copy link
Contributor Author

fizruk commented Jun 29, 2017

@klappvisor I'm currently experimenting with Telegram Bot API using this library. This was the first thing I noticed. However it is a very minor thing.

Recently I noticed that Telegram Bot API simulates sum types:

At most one of the optional parameters can be present in any given update.

This object represents one button of an inline keyboard. You must use exactly one of the optional fields.

Exactly one of the fields data or game_short_name will be present.

telegram-api currently copies data type design and does not leverage sum types for these objects.
And it makes handling updates somewhat painful (without pattern synonyms or lenses) and also allows for invalid data to be constructed (which could be avoided easily with sum types).

In any case, I'm still trying things out and I may have more concrete suggestions later :)

@klappvisor
Copy link
Owner

@fizruk Thank you for your feedback! It's actually very relevant, you are right, but it would require major breaking change. I guess it's better to do it anyway, hopefully together with the others I could find, to avoid constantly breaking API.

As user, what do you think about current order of the arguments in runClient?
Currently:

runClient queries token manager

Or

runClient token manager queries

That will make it possible to write

runClient toekn manager $ do
  me<- getMeM
  webhook <- getWebhookInfoM

Any other suggestions are welcome

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

No branches or pull requests

2 participants