core
core copied to clipboard
Get rid of ServerResponse return type requirement in execute() method
Currently every execute method in any Command class needs to return a ServerResponse object. But nothing is ever done with it, except for a quick check for isOk() in https://github.com/php-telegram-bot/core/blob/ba430516d3525f322c8285befd3433a63eae30bf/src/Telegram.php#L558
We should remove that requirement, since it just forces the user to add a
return Request::emptyResponse();
at the end of the method and prohibits early returns without repeating creates yourself with the emptyResponse, if the method does not need to do something with Telegram. When a Command results in multiple requests to Telegram, you have to device which response you return. This results in an asymmetry. It may be unclear, why one of them is returned, but not the other or something like that.
Open for discussion.
Right, this has come up before and the main reason to keep a "proper" response object was to allow passing back any error that comes from Telegram, which can be accessed via $response->getResult()
.
@TiiFuchs What type should we be returning here instead, what did you have in mind?
@jacklul What's your view on this?
I always thought it made sense to use that return type, though I think it is useless when you make multiple API calls in a single request and then return just one of the results.