Каков наилучший способ рефакторинга метода, который имеет слишком много (6+) параметров?

Question

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

return new Shniz(foo, bar, baz, quux, fred, wilma, barney, dino, donkey)

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

ShnizArgs args = new ShnizArgs(foo, bar, baz, quux, fred, wilma, barney, dino, donkey)
return new Shniz(args);

Так что это не похоже на улучшение. Так каков наилучший подход?

Answer 1

Я предполагаю, что вы имеете в виду C # . Некоторые из этих вещей применимы и к другим языкам.

У вас есть несколько вариантов:

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

class C
{
    public string S { get; set; }
    public int I { get; set; }
}

new C { S = "hi", I = 3 };

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

Образец строителя .

Подумайте об отношениях между string и StringBuilder. Вы можете получить это для своих собственных классов. Мне нравится реализовывать его как вложенный класс, поэтому класс C имеет связанный класс C.Builder. Мне также нравится свободный интерфейс на сборщик. Сделано правильно, вы можете получить синтаксис так:

C c = new C.Builder()
    .SetX(4)    // SetX is the fluent equivalent to a property setter
    .SetY("hello")
    .ToC();     // ToC is the builder pattern analog to ToString()

// Modify without breaking immutability
c = c.ToBuilder().SetX(2).ToC();

// Still useful to have a traditional ctor:
c = new C(1, "...");

// And object initializer syntax is still available:
c = new C.Builder { X = 4, Y = "boing" }.ToC();

У меня есть сценарий PowerShell, который позволяет мне сгенерировать код компоновщика, чтобы сделать все это, где ввод выглядит так:

class C {
    field I X
    field string Y
}

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

Рефакторинг "Ввести объект параметра" . См. Каталог рефакторинга . Идея состоит в том, что вы берете некоторые передаваемые параметры и помещаете их в новый тип, а затем вместо этого передаете экземпляр этого типа. Если вы сделаете это, не задумываясь, вы вернетесь туда, откуда начали:

new C(a, b, c, d);

становится

new C(new D(a, b, c, d));

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

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

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

3. Ищите функциональность, которая есть в существующем коде, но принадлежит новому типу.

Например, может быть, вы видите какой-то код, похожий на:

bool SpeedIsAcceptable(int minSpeed, int maxSpeed, int currentSpeed)
{
    return currentSpeed >= minSpeed & currentSpeed < maxSpeed;
}

Вы можете взять параметры minSpeed и maxSpeed и поместить их в новый тип:

class SpeedRange
{
   public int Min;
   public int Max;
}

bool SpeedIsAcceptable(SpeedRange sr, int currentSpeed)
{
    return currentSpeed >= sr.Min & currentSpeed < sr.Max;
}

Это лучше, но чтобы действительно воспользоваться преимуществами нового типа, перенесите сравнения в новый тип:

class SpeedRange
{
   public int Min;
   public int Max;

   bool Contains(int speed)
   {
       return speed >= min & speed < Max;
   }
}

bool SpeedIsAcceptable(SpeedRange sr, int currentSpeed)
{
    return sr.Contains(currentSpeed);
}

И сейчас мы кое-что получаем: реализация SpeedIsAcceptable() теперь говорит, что вы имеете в виду, и у вас есть полезный, многократно используемый класс. (Следующий очевидный шаг - сделать SpeedRange в Range<Speed>.)

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

Answer 2

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

Например, если вы передаете спецификацию для прямоугольника, вы можете передать x, y, width и height или просто передать объект прямоугольника, который содержит x, y, width и height.

Ищите такие вещи при рефакторинге, чтобы немного его очистить. Если аргументы действительно не могут быть объединены, начните выяснять, есть ли у вас нарушение принципа единой ответственности.

Answer 3

Если это конструктор, особенно если есть несколько перегруженных вариантов, вы должны взглянуть на шаблон Builder:

Foo foo = new Foo()
          .configBar(anything)
          .configBaz(something, somethingElse)
          // and so on

Если это обычный метод, вам следует подумать о связях между передаваемыми значениями и, возможно, создать Transfer Object.

Answer 4

Это цитата из книги Фаулера и Бека: «Рефакторинг»

Длинный список параметров

В наши первые дни программирования нас учили передавать в качестве параметров все, что нужно рутина. Это было понятно, потому что альтернативой были глобальные данные, а глобальные данные злой и обычно болезненный. Объекты меняют эту ситуацию, потому что если у вас нет чего-то вам нужно, вы всегда можете попросить другой объект, чтобы получить его для вас. Таким образом, с объектами вы не передать все, что нужно методу; вместо этого вы передаете достаточно, чтобы метод мог добраться до все, что нужно Многое из того, что нужно методу, доступно в классе хоста метода. В списки параметров объектно-ориентированных программ, как правило, намного меньше, чем в традиционных программы. Это хорошо, потому что длинные списки параметров трудно понять, потому что они становятся непоследовательны и трудны в использовании, и потому что вы навсегда меняете их по мере необходимости больше данных. Большинство изменений удаляются при передаче объектов, потому что вы гораздо более вероятны нужно сделать всего пару запросов, чтобы получить новый фрагмент данных. Используйте параметр Заменить параметр методом, когда вы можете получить данные в одном параметре, сделав запрос объекта, о котором вы уже знаете. Этот объект может быть полем или другой параметр. Используйте Preserve Whole Object, чтобы получить кучу данных, полученных из возразите и замените его самим объектом. Если у вас есть несколько элементов данных без логического объект, используйте Ввести объект параметра. Есть одно важное исключение для внесения этих изменений. Это когда вы явно делаете не хочу создавать зависимость от вызываемого объекта до более крупного объекта. В тех случаях распаковка данных и отправка их в качестве параметров разумна, но обратите внимание на боль участвует. Если список параметров слишком длинный или меняется слишком часто, вам необходимо переосмыслить структура зависимостей.

Answer 5

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

например. вместо:

driver.connect(host, user, pass)

Вы можете использовать

config = new Configuration()
config.setHost(host)
config.setUser(user)
config.setPass(pass)
driver.connect(config)

YMMV

Answer 6

Когда я вижу длинные списки параметров, мой первый вопрос - делает ли эта функция или объект слишком много. Рассмотрим:

EverythingInTheWorld earth=new EverythingInTheWorld(firstCustomerId,
  lastCustomerId,
  orderNumber, productCode, lastFileUpdateDate,
  employeeOfTheMonthWinnerForLastMarch,
  yearMyHometownWasIncorporated, greatGrandmothersBloodType,
  planetName, planetSize, percentWater, ... etc ...);

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

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

 Order order=new Order(customerName, customerAddress, customerCity,
   customerState, customerZip,
   orderNumber, orderType, orderDate, deliveryDate);

Мы могли бы иметь:

Customer customer=new Customer(customerName, customerAddress,
  customerCity, customerState, customerZip);
Order order=new Order(customer, orderNumber, orderType, orderDate, deliveryDate);

Хотя, конечно, я предпочитаю функции, которые принимают всего 1, 2 или 3 параметра, иногда нам приходится принимать, что реально эта функция занимает кучу и что само число не создает сложности. Например:

Employee employee=new Employee(employeeId, firstName, lastName,
  socialSecurityNumber,
  address, city, state, zip);

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

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

void updateCustomer(String type, String status,
  int lastOrderNumber, int pastDue, int deliveryCode, int birthYear,
  int addressCode,
  boolean newCustomer, boolean taxExempt, boolean creditWatch,
  boolean foo, boolean bar);

А потом я вижу, что это называется:

updateCustomer("A", "M", 42, 3, 1492, 1969, -7, true, false, false, true, false);

Я обеспокоен. Глядя на вызов, не совсем понятно, что означают все эти загадочные числа, коды и флаги. Это просто просит ошибок. Программист может легко запутаться в порядке расположения параметров и случайно переключить два, и, если они имеют одинаковый тип данных, компилятор просто примет это. Я бы предпочел иметь подпись, где все эти вещи являются перечислениями, поэтому вызов передается в таких вещах, как Type.ACTIVE вместо «A» и CreditWatch.NO вместо «false» и т. Д.

Answer 7

Я не хочу звучать как мудрый взлом, но вы также должны проверить, чтобы данные, которые вы передаете, действительно должны передаваться: Передача материала в конструктор (или метод в этом отношении) пахнет немного как небольшой акцент на поведении объекта.

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

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

Answer 8

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

return new Shniz.Builder(foo, bar).baz(baz).quux(quux).build();

Подробности этого описаны в Effective Java, 2nd Ed., P. 11. Для аргументов метода в той же книге (стр. 189) описаны три подхода к сокращению списков параметров:

• Разбейте метод на несколько методов, которые принимают меньше аргументов
• Создание статических вспомогательных классов-членов для представления групп параметров, т. Е. Передача DinoDonkey вместо dino и donkey
• Если параметры являются необязательными, приведенный выше конструктор может быть принят для методов, определяя объект для всех параметров, устанавливая требуемые и затем вызывая для него некоторый метод execute

Answer 9

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

Шниз (фу, бар, баз, квукс, фред, вилма, барни, дино, осел)

можно интерпретировать как:

void Shniz(int foo, int bar, int baz, int quux, int fred, 
           int wilma, int barney, int dino, int donkey) { ...

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

// old way
Shniz(1,2,3,2,3,2,1,2);
Shniz(1,2,2,3,3,2,1,2); 

//versus
ShnizParam p = new ShnizParam { Foo = 1, Bar = 2, Baz = 3 };
Shniz(p);

В качестве альтернативы, если у вас было:

void Shniz(Foo foo, Bar bar, Baz baz, Quux quux, Fred fred, 
           Wilma wilma, Barney barney, Dino dino, Donkey donkey) { ...

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

Кроме того, некоторые параметры являются необязательными? Существуют ли переопределения метода (то же имя метода, но разные сигнатуры метода?) Все эти подробности имеют значение в отношении ответа best .

* Пакет свойств также может быть полезен, но не лучше, если учесть, что фон не задан.

Как видите, существует более 1 правильного ответа на этот вопрос. Выбирай.

Answer 10

Я бы использовал конструктор по умолчанию и установщики свойств. В C # 3.0 есть хороший синтаксис, позволяющий делать это автоматически.

return new Shniz { Foo = foo,
                   Bar = bar,
                   Baz = baz,
                   Quuz = quux,
                   Fred = fred,
                   Wilma = wilma,
                   Barney = barney,
                   Dino = dino,
                   Donkey = donkey
                 };

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