Обоснование предпочитать локальные переменные переменным экземпляра?

109

Кодовая база, над которой я работаю, часто использует переменные экземпляра для обмена данными между различными тривиальными методами. Первоначальный разработчик непреклонен, что он придерживается лучших практик, изложенных в книге « Чистый код » дядюшки Боба / Роберта Мартина: «Первое правило функций заключается в том, что они должны быть маленькими». и «Идеальное число аргументов для функции - ноль (нильадик). (...) Аргументы - это сложно. Они требуют много концептуальной силы».

Пример:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  private byte[] encodedData;
  private EncryptionInfo encryptionInfo;
  private EncryptedObject payloadOfResponse;
  private URI destinationURI;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    getEncodedData(encryptedRequest);
    getEncryptionInfo();
    getDestinationURI();
    passRequestToServiceClient();

    return cryptoService.encryptResponse(payloadOfResponse);
  }

  private void getEncodedData(EncryptedRequest encryptedRequest) {
    encodedData = cryptoService.decryptRequest(encryptedRequest, byte[].class);
  }

  private void getEncryptionInfo() {
    encryptionInfo = cryptoService.getEncryptionInfoForDefaultClient();
  }

  private void getDestinationURI() {
    destinationURI = router.getDestination().getUri();
  }

  private void passRequestToServiceClient() {
    payloadOfResponse = serviceClient.handle(destinationURI, encodedData, encryptionInfo);
  }
}

Я бы преобразовал это в следующее, используя локальные переменные:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    byte[] encodedData = cryptoService.decryptRequest(encryptedRequest, byte[].class);
    EncryptionInfo encryptionInfo = cryptoService.getEncryptionInfoForDefaultClient();
    URI destinationURI = router.getDestination().getUri();
    EncryptedObject payloadOfResponse = serviceClient.handle(destinationURI, encodedData,
      encryptionInfo);

    return cryptoService.encryptResponse(payloadOfResponse);
  }
}

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

Отсюда мои вопросы: какова цель, научное обоснование предпочтения локальных переменных переменным экземпляра? Кажется, я не могу понять, как это сделать. Моя интуиция говорит мне, что скрытые связи плохие и что узкая область лучше, чем широкая. Но какая наука поддерживает это?

И наоборот, есть ли недостатки этого рефакторинга, которые я, возможно, упустил из виду?

Александр
источник
Комментарии не для расширенного обсуждения; этот разговор был перемещен в чат .
maple_shaft

Ответы:

170

Какова цель, научное обоснование предпочтения локальных переменных переменным экземпляра?

Сфера - это не двоичное состояние, а градиент. Вы можете ранжировать их от самых больших до самых маленьких:

Global > Class > Local (method) > Local (code block, e.g. if, for, ...)

Изменить: то, что я называю «область видимости класса», это то, что вы подразумеваете под «переменной экземпляра». Насколько мне известно, это синонимы, но я разработчик C #, а не разработчик Java. Ради краткости я объединил всю статику в глобальную категорию, поскольку статика не является темой вопроса.

Чем меньше сфера, тем лучше. Обоснование состоит в том, что переменные должны жить в наименьшем возможном объеме . Есть много преимуществ для этого:

  • Это заставляет вас думать об ответственности текущего класса и помогает вам придерживаться SRP.
  • Это позволит вам не избежать глобальных конфликтов имен, например , если два или более классов имеют Nameсвойство, вы не принуждали префикс их , как FooName, BarName... Таким образом , сохраняя ваши имена переменных , как чистый и лаконичный , насколько это возможно.
  • Он обезвреживает код, ограничивая доступные переменные (например, для Intellisense) теми, которые имеют контекстное значение.
  • Он обеспечивает некоторую форму контроля доступа, поэтому вашими данными не может управлять незнакомый вам актер (например, другой класс, разработанный коллегой).
  • Это делает код более читабельным, так как вы гарантируете, что объявление этих переменных пытается оставаться как можно ближе к фактическому использованию этих переменных.
  • Бессмысленное объявление переменных в слишком широкой области часто указывает на разработчика, который не совсем понимает ООП или как его реализовать. Наблюдение за слишком широкими областями видимости действует как красный флаг, что, вероятно, что-то идет не так с подходом ООП (либо с разработчиком в целом, либо с базой кода в частности).
  • (Комментарий Кевина) Использование местных жителей заставляет вас делать вещи в правильном порядке. В исходном коде (переменная класса) вы могли ошибочно перейти passRequestToServiceClient()в начало метода, и он все равно будет компилироваться. С локальными пользователями вы могли бы совершить эту ошибку, только если передали неинициализированную переменную, что, как мы надеемся, достаточно очевидно, что вы на самом деле этого не делаете.

Тем не менее, несмотря на эти преимущества, я все еще не могу убедить первоначального разработчика в том, что этот рефакторинг оправдан, так как, по-видимому, он противоречит практике дяди Боба, упомянутой выше.

И наоборот, есть ли недостатки этого рефакторинга, которые я, возможно, упустил из виду?

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

Хотя я понимаю ваше предложение «без переменной класса» и в этом есть смысл, вы на самом деле также удалили сами методы, и это совершенно другая игра. Методы должны были остаться, и вместо этого вы должны были изменить их, чтобы они возвращали свое значение, а не сохраняли его в переменной класса:

private byte[] getEncodedData() {
    return cryptoService.decryptRequest(encryptedRequest, byte[].class);
}

private EncryptionInfo getEncryptionInfo() {
    return cryptoService.getEncryptionInfoForDefaultClient();
}

// and so on...

Я согласен с тем, что вы сделали в processметоде, но вы должны были вызывать закрытые субметоды, а не выполнять их тела напрямую.

public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    byte[] encodedData = getEncodedData();
    EncryptionInfo encryptionInfo = getEncryptionInfo();

    //and so on...

    return cryptoService.encryptResponse(payloadOfResponse);
}

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

Независимо от аргумента локальной переменной, я сразу заметил, что предложенное вами исправление значительно менее читабельно, чем оригинал. Я признаю, что бессмысленное использование переменных класса также снижает читабельность кода, но не на первый взгляд, по сравнению с тем, как вы сложили всю логику в одном (теперь многословном) методе.

Flater
источник
Комментарии не для расширенного обсуждения; этот разговор был перемещен в чат .
maple_shaft
79

Исходный код использует переменные-члены, такие как аргументы. Когда он говорит минимизировать количество аргументов, он на самом деле имеет в виду минимизацию количества данных, которое требуется методам для функционирования. Помещение этих данных в переменные-члены ничего не улучшает.

Alex
источник
20
Полностью согласен! Эти переменные-члены являются просто неявными аргументами функции. На самом деле это еще хуже, поскольку теперь нет явной связи между значением этой переменной и использованием функции (из внешнего POV)
Rémi
1
Я бы сказал, что это не то, что имела в виду книга. Сколько функций требует ровно нулевых входных данных для запуска? Это один из кусочков, я думаю, что книга ошиблась.
Qwertie
1
@Qwertie Если у вас есть объект, данные, с которыми он работает, могут быть полностью инкапсулированы внутри него. Такие функции, как process.Start();или myString.ToLowerCase()не должны казаться слишком странными (и, действительно, их легче понять).
Р. Шмитц
5
У обоих есть один аргумент: неявный this. Можно даже утверждать, что этот аргумент приводится явно - до точки.
Блэкджек
47

Другие ответы уже прекрасно объяснили преимущества локальных переменных, так что все, что осталось, это часть вашего вопроса:

Тем не менее, несмотря на эти преимущества, я все еще не могу убедить первоначального разработчика в том, что этот рефакторинг оправдан, так как, по-видимому, он противоречит практике дяди Боба, упомянутой выше.

Это должно быть легко. Просто укажите ему следующую цитату в Чистом коде дяди Боба:

Не имеет побочных эффектов

Побочные эффекты - ложь. Ваша функция обещает сделать одну вещь, но она также делает другие скрытые вещи. Иногда он будет вносить неожиданные изменения в переменные своего собственного класса. Иногда они соответствуют параметрам, переданным в функцию или системным глобальным переменным. В любом случае они являются коварными и разрушительными неправдами, которые часто приводят к странным временным связям и порядковым зависимостям.

(пример опущен)

Этот побочный эффект создает временную связь. То есть checkPassword может быть вызван только в определенные моменты времени (другими словами, когда безопасно инициализировать сеанс). Если он вызывается не по порядку, данные сеанса могут быть случайно потеряны. Временные связи сбивают с толку, особенно когда они скрыты как побочный эффект. Если у вас должна быть временная связь, вы должны указать это в названии функции. В этом случае мы могли бы переименовать функцию checkPasswordAndInitializeSession, хотя это, безусловно, нарушает «Делать одну вещь».

То есть дядя Боб не просто говорит, что функция должна принимать несколько аргументов, он также говорит, что функции должны по возможности избегать взаимодействия с нелокальным состоянием.

Meriton
источник
3
В «мире префектов» это будет второй из перечисленных ответов. Первый ответ для идеальной ситуации, которую коллега прислушивается к разуму, - но если коллега является фанатиком, этот ответ здесь будет иметь дело с ситуацией без особых затруднений.
Р. Шмитц
2
Для более прагматичного варианта этой идеи, просто гораздо проще рассуждать о локальном состоянии, чем об экземпляре или глобальном состоянии. Четко определенные и жестко ограниченные изменчивость и побочные эффекты редко приводят к проблемам. Например, многие функции сортировки работают на месте через побочный эффект, но об этом легко рассуждать в локальной области.
Beefster
1
Ах, старый добрый "в противоречивой аксиоме, можно доказать что угодно". Поскольку не существует строгих истин и лжи IRL, любые догмы должны будут включать утверждения, которые утверждают противоположные вещи, то есть противоречивы.
ivan_pozdeev
26

«Это противоречит тому, что думает чей-то дядя», НИКОГДА не хороший аргумент. НИКОГДА. Не бери мудрости у дяди, думай сам.

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

Тест: написать комментарий к документации для каждой переменной экземпляра. Вы можете написать что-нибудь, что не является абсолютно бессмысленным? И написать комментарий к документации для четырех участников. Они одинаково бессмысленны.

Хуже всего, предположите способ расшифровки изменений, потому что вы используете другой криптосервис. Вместо того, чтобы менять четыре строки кода, вы должны заменить четыре переменные экземпляра на разные, четыре метода получения на разные и изменить четыре строки кода.

Но, конечно, первая версия предпочтительнее, если вам платит строка кода. 31 строка вместо 11 строк. В три раза больше строк для записи и для сохранения навсегда, для чтения при отладке чего-либо, для адаптации при необходимости изменений, для дублирования при поддержке второго cryptoService.

(Пропустил важный момент: использование локальных переменных заставляет вас делать вызовы в правильном порядке).

gnasher729
источник
16
Мышление для себя, конечно, хорошо. Но ваш вступительный абзац фактически включает учеников или юниоров, отклоняющих вклад учителей или пожилых людей; что заходит слишком далеко.
Флейтер
9
@Flater Подумав о вкладе учителей или пожилых людей и увидев, что они не правы, отклонить их вклад - единственно правильная вещь. В конце концов, речь идет не об увольнении, а о допросе и увольнении, только если оно определенно окажется неправильным.
Glglgl
10
@ChristianHackl: Я полностью согласен с тем, чтобы не слепо следовать традиционализму, но я также не буду слепо отмахиваться от него. Ответ, по-видимому, предлагает отказаться от приобретенных знаний в пользу вашего собственного взгляда, а это не здоровый подход. Слепого следования чужим мнениям нет. Задавая вопросы, когда вы не согласны, очевидно, да. Откровенно отклонить это, потому что вы не согласны, нет. Насколько я понимаю, первый абзац ответа, по крайней мере , подсказывает последний. Это очень сильно зависит от того, что означает gnasher с «думай сам», что требует доработки.
Флейтер
9
На платформе обмена мудростью это кажется немного неуместным ...
drjpizzle
4
НИКОГДА также НИКОГДА не является хорошим аргументом ... Я полностью согласен с тем, что существуют правила для руководства, а не для назначения. Тем не менее, правила о правилах являются лишь подклассом правил, так что вы высказали свой собственный вердикт ...
cmaster
14

Какова цель, научное обоснование предпочтения локальных переменных переменным экземпляра? Кажется, я не могу понять, как это сделать. Моя интуиция говорит мне, что скрытые связи плохие и что узкая область лучше, чем широкая. Но какая наука поддерживает это?

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

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

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

Джон Боллинджер
источник
7
Пока что это единственный ответ, в котором упоминается безопасность потоков и параллелизм. Это удивительно, учитывая конкретный пример кода в вопросе: экземпляр SomeBusinessProcess не может безопасно обрабатывать несколько зашифрованных запросов одновременно. Метод public EncryptedResponse process(EncryptedRequest encryptedRequest)не синхронизирован, и параллельные вызовы вполне могут привести к засорению значений переменных экземпляра. Это хороший момент для воспитания.
Джошуа Тейлор
9

Обсуждая справедливо process(...), пример ваших коллег гораздо более разборчив в смысле бизнес-логики. И наоборот, ваш встречный пример требует не только беглого взгляда, чтобы извлечь какой-либо смысл.

При этом чистый код является и разборчивым, и хорошего качества - выталкивание локального состояния в более глобальное пространство - это просто сборка высокого уровня, поэтому ноль для качества.

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest request) {
    checkNotNull(encryptedRequest);

    return encryptResponse
      (routeTo
         ( destination()
         , requestData(request)
         , destinationEncryption()
         )
      );
  }

  private byte[] requestData(EncryptedRequest encryptedRequest) {
    return cryptoService.decryptRequest(encryptedRequest, byte[].class);
  }

  private EncryptionInfo destinationEncryption() {
    return cryptoService.getEncryptionInfoForDefaultClient();
  }

  private URI destination() {
    return router.getDestination().getUri();
  }

  private EncryptedObject routeTo(URI destinationURI, byte[] encodedData, EncryptionInfo encryptionInfo) {
    return serviceClient.handle(destinationURI, encodedData, encryptionInfo);
  }

  private void encryptResponse(EncryptedObject payloadOfResponse) {
    return cryptoService.encryptResponse(payloadOfResponse);
  }
}

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

Просто пункт о наименовании. Вы хотите самое короткое имя, которое имеет смысл и расширяет доступную информацию. то есть. destinationURI, URI уже известен по сигнатуре типа.

Kain0_0
источник
4
Исключение всех переменных не обязательно облегчает чтение кода.
Pharap
Полностью исключите все переменные в стиле pointfree en.wikipedia.org/wiki/Tacit_programming
Марцин
@Pharap Правда, отсутствие переменных не обеспечивает разборчивость. В некоторых случаях это даже затрудняет отладку. Дело в том, что правильно выбранные имена, четкое использование выражений, могут очень четко донести идею, оставаясь при этом эффективными.
Kain0_0
7

Я бы просто удалил эти переменные и приватные методы. Вот мой рефакторинг:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    return cryptoService.encryptResponse(
        serviceClient.handle(router.getDestination().getUri(),
        cryptoService.decryptRequest(encryptedRequest, byte[].class),
        cryptoService.getEncryptionInfoForDefaultClient()));
  }
}

Для частного метода, например router.getDestination().getUri(), более понятный и более читаемый, чем getDestinationURI(). Я бы даже повторил это, если бы я использовал одну и ту же строку дважды в одном классе. Если взглянуть на это иначе, если есть необходимость getDestinationURI(), то он, вероятно, принадлежит другому классу, а не SomeBusinessProcessклассу.

Для переменных и свойств общая потребность в них состоит в том, чтобы хранить значения, которые будут использоваться позже во времени. Если у класса нет открытого интерфейса для свойств, они, вероятно, не должны быть свойствами. Худший вид использования свойств класса, вероятно, для передачи значений между закрытыми методами посредством побочных эффектов.

В любом случае, классу нужно только сделать, process()и тогда объект будет отброшен, нет необходимости сохранять какое-либо состояние в памяти. Дальнейший потенциал рефакторинга будет заключаться в том, чтобы вывести CryptoService из этого класса.

Основываясь на комментариях, я хочу добавить, что этот ответ основан на реальной мировой практике. Действительно, в обзоре кода первое, что я бы выбрал, - это рефакторинг класса и удаление работы шифрования / дешифрования. Как только это будет сделано, я бы спросил, нужны ли методы и переменные, правильно ли они названы и так далее. Окончательный код, вероятно, будет ближе к этому:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;

  public Response process(Request request) {
    return serviceClient.handle(router.getDestination().getUri());
  }
}

С приведенным выше кодом, я не думаю, что это требует дальнейшего рефакторинга. Как и в случае с правилами, я думаю, что требуется опыт, чтобы знать, когда и когда их не применять. Правила - это не теории, доказавшие свою эффективность во всех ситуациях.

С другой стороны, проверка кода оказывает реальное влияние на то, как долго может пройти кусок кода. Моя хитрость заключается в том, чтобы иметь меньше кода и сделать его легким для понимания. Имя переменной может быть предметом обсуждения, если я смогу удалить его, рецензентам даже не нужно будет об этом думать.

imel96
источник
Мое мнение, хотя многие здесь будут колебаться. Конечно, некоторые абстракции имеют смысл. (Кстати, метод под названием «процесс» ужасен.) Но здесь логика минимальна. Вопрос OP, однако, касается всего стиля кода, и в этом случае дело может быть более сложным.
Joop Eggen
1
Одна очевидная проблема с объединением всего этого в один вызов метода - просто читаемость. Это также не работает, если вам нужно более одной операции над данным объектом. Кроме того, это почти невозможно отладить, потому что вы не можете выполнять операции и проверять объекты. Хотя это работает на техническом уровне, я бы не стал выступать за это, поскольку он в значительной степени игнорирует аспекты разработки программного обеспечения, не связанные со временем выполнения.
Flater
@Flater Я согласен с вашими комментариями, мы не хотим применять это везде. Я отредактировал свой ответ, чтобы уточнить мою практическую позицию. Я хочу показать, что на практике мы применяем правила только тогда, когда это уместно. В этом случае вызов метода цепочки - это нормально, и если мне нужно отладить, я бы вызвал тесты для связанных методов.
imel96
@JoopEggen Да, абстракции имеют смысл. В этом примере частные методы не дают никаких абстракций в любом случае, пользователи класса даже не знают о них
imel96
1
@ imel96 забавно, что вы, вероятно, один из немногих людей, которые заметили здесь, что связь между ServiceClient и CryptoService делает целесообразным сосредоточиться на внедрении CS в SC вместо SCP, решая основную проблему на более высоком архитектурном уровне ... это IMVHO основной смысл этой истории; слишком легко отслеживать общую картину, сосредотачиваясь на деталях.
vaxquis
4

Ответ Флэтера достаточно хорошо охватывает вопросы определения объема, но я думаю, что здесь есть и другая проблема.

Обратите внимание, что есть разница между функцией, которая обрабатывает данные, и функцией, которая просто обращается к данным .

Первый выполняет реальную бизнес-логику, тогда как второй экономит набор текста и, возможно, повышает безопасность, добавляя более простой и более повторно используемый интерфейс.

В этом случае кажется, что функции доступа к данным не сохраняют набор текста и нигде не используются повторно (или могут возникнуть другие проблемы при их удалении). Так что эти функции просто не должны существовать.

Сохраняя только бизнес - логику названных функций, мы получаем лучшее из обоих миров (где - то между ответом Flater в и ответ imel96 в ):

public EncryptedResponse process(EncryptedRequest encryptedRequest) {

    byte[] requestData = decryptRequest(encryptedRequest);
    EncryptedObject responseData = handleRequest(router.getDestination().getUri(), requestData, cryptoService.getEncryptionInfoForDefaultClient());
    EncryptedResponse response = encryptResponse(responseData);

    return response;
}

// define: decryptRequest(), handleRequest(), encryptResponse()
user673679
источник
3

Первая и самая важная вещь: дядя Боб, кажется, иногда похож на проповедника, но утверждает, что есть исключения из его правил.

Вся идея Чистого кода состоит в том, чтобы улучшить читаемость и избежать ошибок. Есть несколько правил, которые нарушают друг друга.

Его аргумент в отношении функций заключается в том, что функции niladic являются лучшими, однако допустимо до трех параметров. Я лично думаю, что 4 тоже в порядке.

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

Переменные, которые не используются во многих местах класса, должны быть перемещены.

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

кап
источник
1

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

Переменная экземпляра уменьшает способы вызова функции, что также помогает уменьшить определенные классы ошибок и улучшает удобочитаемость.

Сказать, что одно правильно, а другое неправильно, вполне может быть верным выводом в любом конкретном случае, но в качестве общего совета ...

TL; DR: Я думаю, что причина того, что ты слишком сильно пахнешь, в том, что слишком много рвения.

drjpizzle
источник
0

Несмотря на то, что методы, начинающиеся с get ..., не должны возвращать void, разделение уровней абстракций внутри методов дано в первом решении. Хотя второе решение более ограничено, все еще сложнее рассуждать о том, что происходит в методе. Назначения локальных переменных здесь не нужны. Я бы сохранил имена методов и изменил бы код на что-то вроде этого:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    checkNotNull(encryptedRequest);

    return getEncryptedResponse(
            passRequestToServiceClient(getDestinationURI(), getEncodedData(encryptedRequest) getEncryptionInfo())
        );
  }

  private EncryptedResponse getEncryptedResponse(EncryptedObject encryptedObject) {
    return cryptoService.encryptResponse(encryptedObject);
  }

  private byte[] getEncodedData(EncryptedRequest encryptedRequest) {
    return cryptoService.decryptRequest(encryptedRequest, byte[].class);
  }

  private EncryptionInfo getEncryptionInfo() {
    return cryptoService.getEncryptionInfoForDefaultClient();
  }

  private URI getDestinationURI() {
    return router.getDestination().getUri();
  }

  private EncryptedObject passRequestToServiceClient(URI destinationURI, byte[] encodedData, EncryptionInfo encryptionInfo) {
    return serviceClient.handle(destinationURI, encodedData, encryptionInfo);
  }
}
Роман Вейс
источник
0

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

И мне тоже нравится ваш путь лучше, чем ваш коллега. Почему? Потому что я думаю, что легче читать и понимать, несмотря на то, что говорит автор какой-то книги.

Оба пути выполняют одно и то же, но его путь более распространен. Чтобы прочитать этот код, вам нужно переключаться между несколькими функциями и переменными-членами. Это не все сжато в одном месте, вам нужно запомнить все это в своей голове, чтобы понять это. Это намного большая когнитивная нагрузка.

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

Однако, если он привык к такому кодированию, я могу представить, что для него это может быть наоборот.

Vilx-
источник
Это действительно оказалось серьезным препятствием для меня, чтобы понять поток. Этот пример довольно небольшой, но другой использует 35 (!) Переменных экземпляра для промежуточных результатов, не считая дюжины переменных экземпляра, содержащих зависимости. За ним было довольно трудно следовать, так как вы должны следить за тем, что уже было установлено ранее. Некоторые из них даже были повторно использованы позже, делая это еще сложнее. К счастью, приведенные здесь аргументы окончательно убедили моего коллегу согласиться с рефакторингом.
Александр