Telegram Rest API client design

I'm working on a "framework" for Telegram bots and right now everything "just works". I'm now in the process of rethinking about what I wrote and one of the things I dislike a lot is how I implemented the API client.

Right now I have an interface (TelegramBotApi) which maps every API endpoint to a method. This interface is implemented by this class:

```
public class TelegramBotRestApi implements TelegramBotApi {

private final ObjectMapper mapper;
private final RestTemplate restTemplate;

private final UriTemplate apiUriTemplate;
private final UriTemplate fileUriTemplate;

public TelegramBotRestApi(String token, ObjectMapper mapper, RestTemplate restTemplate) {
this.mapper = mapper;
this.restTemplate = restTemplate;
this.apiUriTemplate = new UriTemplate("https://api.telegram.org/bot" + token + "/{method}");
this.fileUriTemplate = new UriTemplate("https://api.telegram.org/file/bot" + token + "/{file_path}");
}

@Override
public User getMe() throws TelegramBotApiException {

TelegramBotRestApiCall.Builder builder = new TelegramBotRestApiCall.Builder("getMe", apiUriTemplate,
mapper, restTemplate, User.class);

builder.setHttpMethod(HttpMethod.GET);

return builder.build().call();

}

@Override
public List getUpdates(Integer offset, Integer limit, Integer timeout) throws TelegramBotApiException {

TelegramBotRestApiCall.Builder builder = new TelegramBotRestApiCall.Builder("getUpdates",
apiUriTemplate, mapper, restTemplate, Update[].class);

builder.setParam("offset", offset, false).setParam("limit", limit, false).setParam("timeout", timeout, false);

builder.setContentType(MediaType.MULTIPART_FORM_DATA);

Update[] result = builder.build().call();
Arrays.sort(result);

return Collections.unmodifiableList(Arrays.asList(result));
}

@Override
public Message sendMessage(ChatId chatId, String text, ParseMode parseMode, Boolean disableWebPagePreview,
Boolean disableNotification, Integer re

You are right, the code looks messy. Here are some hints that concern only the fragments listed in the question (I looked briefly inside the project repo, but it's too huge to analyze here).

TelegramBotRestApiCall

-
The multiple raw type warnings can be avoided easily by adding a generic type parameter to payloadType field, making it Class payloadType, with similar changes to the arguments of the two constructors.

-
In this particular case I do not like the idea of typeRefs map for holding ParameterizedTypeReference instances. Looking inside the definition of this object, which is quite lightweight, I think that there is no reason to pre-instantiate them for each class. These instances can be created on-the-fly when necessary. The main drawback of this typeRefs collection is the evolution of API: each time when you (or someone else) add a new object into your data model, initializeParameterizedTypeReferences method will need to be updated and this is not very obvious to find.

I'd suggest to replace this method with just

public static  ParameterizedTypeReference> getTypeReference(Class targetClass) {
  return new ParameterizedTypeReference>() {};
}

-
The builder pattern implementation looks ugly. There are as many constructor arguments in the builder object, as in the target one, while one of the ideas of the pattern is exactly avoiding to have too many constructor arguments. So, there should be methods like method(String method), uriTemplate(UriTemplate uriTemplate), mapper(ObjectMapper mapper) etc. Setter-like methods as setContentType should also be renamed to contentType. I'd keep the set prefix only for setParam method, because the target parameter is passed by name in the arguments.

-
The builder class definition itself can be extracted into separate file. This will allow to eliminate long declarations like TelegramBotRestApiCall.Builder, replacing them with something like ApiCallBuilder. Or import Builder directly into the classes using it.

-
The call method also looks messy, especially due to the nested try-catches, which is something to avoid. The IOException caught in the inner block is lost without trace; the assignment of res is useless because of the exception thrown in the block.

So I can suggest this simplification to the call method:

public T call() throws TelegramBotApiException {
  try {
    final Response res = rest.exchange(uriTemplate.expand(method), httpMethod, entity, getTypeReference(payloadType))
          .getBody();
    if (!res.ok) {
      throw new TelegramBotApiException(res.description, res.errorCode);
    }
    return res.result;
  } catch (HttpStatusCodeException he) {
    throw new TelegramBotApiException(he, 
                                      he.getStatusCode() != null ? he.getStatusCode().value() : null);
  } catch (RestClientException e) {
    throw new TelegramBotApiException(e);
  }
}

TelegramBotRestApi

-
The bodies of the methods should be changed after refactoring the Builder class as mentioned above.

-
URL parameters for UriTemplates construction should be extracted into dedicated (.properties, .json, .conf etc) files and loaded from there. If someday https://api.telegram.org/bot changes to https://api.telegram.org/v1/bot, you won't be pleased to recompile everything, instead of just changing the configuration file.

-
I've had a look into TelegramBotApi interface and I can say that there are many things to improve, for instance:

1) There are too many methods doing very different things. It should be split into smaller ones, by action semantics: user management, sending resources, messaging, etc.

2) Too many arguments for the majority of the methods. Try to reduce them by wrapping things into dedicated objects.

3) There are many methods like:

Message sendVideo(ChatId chatId, Resource video, Integer duration, Integer width, Integer height, String caption,
        Boolean disableNotification, Integer replyToMessageId, AbstractKeyboardMarkup replyMarkup) throws TelegramBotApiException;

Message sendVoice(ChatId chatId, Resource voice, Integer duration, Boolean disableNotification, Integer replyToMessageId, AbstractKeyboardMarkup replyMarkup) throws TelegramBotApiException;

They have ate least five similar arguments. It really seems that instead of having this pullulation of methods there should be an abstraction like send(chatId, resource, etc).

StackExchange Code Review Q#136115, answer score: 2