VKCOM / vk-java-sdk

Java library for working with VK API
MIT License
294 stars 155 forks source link

Добавьте метод setLogger к классу ApiRequest #267

Open wtlgo opened 2 years ago

wtlgo commented 2 years ago

Здравствуйте,

Пожалуйста, добавьте метод setLogger к классу ApiRequest.

Текущая имплементация устроена так, что если в программе уже есть log4j2, то VK SDK к нему обзательно хукается и начинает писать стэктрейсы в лог. Это вызывает тонну дискомфорта при использовании методов, которые зависят от ошибок для получения результата.

Например, возьмем метод groups.getBanned. Если пользователь не забанен в группе, то при вызове этого метода выбросится исключение, стектрейс которого обязательно попадет в лог, хотя, если логически посудить, это не является ошибкой, а вполне себе ожидаемым поведением. Тем не менее, воспрепятствовать засорению лога банально невозможно, так как класс создает себе логгер сам.

public abstract class ApiRequest<T> {
    private static final Logger LOG = LoggerFactory.getLogger(ApiRequest.class);
// ...

Изменение логгера через файлы конфигураций, как это описано в README.md, абсолютно не практично, так как логгер программы сам на них полагается и приходится выбирать между не иметь логи вообще, либо иметь, но с горой мусора из VK SDK. В добавок, мне кажется, это не совсем адекватный дизайн библиотеки, если под нее приходится вносить настолько координальные изменения в программу. В конце концов, VK SDK - это не библиотека для засорения логов, а библиотека для доступа к VK API, поэтому, было бы здорово, если б она выполняла свою работу без подобных сайд-эффектов, а пользователю не приходилось строить дизайн своей программы вокруг этих сайд-эффектов.

Итак, мое предложение - это добавить нестатичное не финальное поле logger, и потом уже использовать этот объект для записи логов вместо LOG.

public abstract class ApiRequest<T> {
    private static final Logger LOG = LoggerFactory.getLogger(ApiRequest.class);
    private Logger logger = LOG;
// ...

   public ApiRequest<T> setLogger(Logger logger) {
       if (logger != null) {
           this.logger = logger;
       }

       return this;
   }
// ...

Это, конечно же, не идеальное решение проблемы, ибо в идеале этот метод должен быть у класса VkApiClient, который бы в свою очередь передавал этот логгер соответствующим объектам, таким образом получилось бы, что это именно клиент ответственный за то, что и как логируется, а не отдельные методы, но такое изменение было бы довольно трудоемким и с текущим дизайном библиотеки потребовало бы довольно много однообразных изменений.

Спасибо за внимание.