rubenlagus / TelegramBots

Java library to create bots using Telegram Bots API
https://telegram.me/JavaBotsApi
MIT License
4.72k stars 1.19k forks source link

API inconsistency: chat id getter as Long vs. chat id setter as String #1065

Open McPringle opened 2 years ago

McPringle commented 2 years ago

I do a lot of conference talks and today I'm updating my Telegram Bot API workshop (I postponed my talks and workshops for about two and a half years). Previously working chatbot examples broke with a compile error. So I went through my examples and was looking for incompatibilities to be updated.

This one was pretty annoying:

@Override
public void onUpdateReceived(final Update update) {
    if (update.hasMessage() && update.getMessage().hasText()) {
        final var message = update.getMessage();
        final var chatId = message.getChatId();   //   <-- chat id is of type Long
        final var text = message.getText();
        final var answer = new SendMessage();
        answer.setChatId(chatId);                 //   <-- compile error, param needs to be of type String
        answer.setText(text);
        try {
            execute(answer);
        } catch (final TelegramApiException e) {
            e.printStackTrace();
        }
    }
}

I was searching for about an hour, decompiling class files and comparing, looking through your source, etc. to figure out what was going wrong here. I reset the index of my IntelliJ IDEA, I even went to the command line because I would never ever think of an API working with different object types for the same value. I ended up finding an issue (for a different class) explaining the Long vs. String problem. To be honest, I was a bit pi**ed.

If you want to accept a chat id or a username, please do not force the users of your API to convert values they got from your API before feeding them back into your API. Either create two methods (one accepting Long values, one accepting String values) or, in my opinion much better, have the setChatId method accept Long values and name the other method something like setChatUsername or similar.

Now I need to explain these inconsistencies to my talk and workshop attendees and I know, there will be discussions about it... 😒

addo47 commented 2 years ago

https://core.telegram.org/bots/api#sendmessage

This chat ID variable accepts a string for channel usernames and an integer for chat IDs. Obviously, only one can superset the other.

Using setChatUsername for strings would make the code more writable, but it will lose its consistency with the setX - JSON field paradigm. Given TG expects this field to be chat_id with a string variable for usernames, you're setting chat_id, not chat_username.

It's tricky; there's no silver bullet. I'm in neither camps.

Chase22 commented 2 years ago

If anything overloading would be the way to go here. The Library is a close replication of the Api and the API returns chatIds as Longs and accepts chatIds as Strings or Longs. The Long setter could be as simple as calling the String setter with a converted value.

rubenlagus commented 2 years ago

@McPringle @Chase22 @addo47 this should be fixed now in Dev branch, please check and confirm