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

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

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

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

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

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

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

Шаблона.

подумайте об отношениях между string и StringBuilder. Вы можете получить это для своих собственных классов. Мне нравится реализовывать его как вложенный класс, поэтому class 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>.)

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

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

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

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

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

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

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

driver.connect(host, user, pass)

вы могли бы использовать

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

YMMV

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

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

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

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

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);

меня беспокоит. Глядя на звонок, совсем не ясно, что означают все эти загадочные цифры, коды и флаги. Это просто напрашивается на ошибки. Программист может легко запутаться в порядке параметров и случайно переключить два, и если они имеют один и тот же тип данных, компилятор просто примет его. Я бы предпочел иметь подпись, где все эти вещи перечислений, поэтому вызов проходит в таких вещах, как тип.Активный вместо "А" и списка CreditWatch.Нет, вместо "ложно" и т. д.

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

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

подробности этого описаны в эффективной Java, 2-е изд. С. 11. Для аргументов метода в той же книге (стр. 189) описаны три подхода к сокращению списков параметров:

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

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

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

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

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

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

может быть истолковано как:

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) { ...

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

кроме того, некоторые параметры необязательные? Есть ли переопределение метода (то же имя метода, но другое сигнатуры метода?) Такого рода детали все дело в том, что лучшие ответ.

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

как вы можете видеть, есть больше, чем 1 Правильный ответ на этот вопрос. Выбирай сам.

вы можете попытаться сгруппировать свой параметр в кратные значимые структуры / классы (если это возможно).

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

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

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

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

Как насчет того, чтобы не устанавливать его сразу в конструкторах, но делать это через свойства/сеттеры? Я видел некоторые классы .NET, которые используют этот подход, такие как Process класс:

        Process p = new Process();

        p.StartInfo.UseShellExecute = false;
        p.StartInfo.CreateNoWindow = true;
        p.StartInfo.RedirectStandardOutput = true;
        p.StartInfo.RedirectStandardError = true;
        p.StartInfo.FileName = "cmd";
        p.StartInfo.Arguments = "/c dir";
        p.Start();

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

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

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

предпочитают небольшие классы / методы над большими. Помните о принципе единой ответственности.

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

другой вариант, который я бы искал при выполнении такого рефактора, - это группы связанных параметров, которые могут быть лучше обработаны как независимый объект. Используя класс Rectangle из более раннего ответа в качестве примера, конструктор, который принимает параметры для x, y, height и width, может разложить x и y на точечный объект, что позволит вам передать три параметра конструктору прямоугольника. Или пойти немного дальше и сделать два параметра (UpperLeftPoint, LowerRightPoint), но это будет более радикальный рефакторинг.

Это зависит от того, какие аргументы у вас есть, но если они много логических значений/опций, возможно, вы могли бы использовать перечисление флага?

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

в некоторых случаях 7-параметровый конструктор может указывать на плохую иерархию классов: в этом случае вспомогательная структура/класс, предложенная выше, обычно является хорошим подходом, но тогда вы также склонны в конечном итоге иметь множество структур, которые являются просто мешками свойств и не делают ничего полезного. Конструктор с 8 аргументами также может указывать на то, что ваш класс слишком общий / слишком универсальный, поэтому ему нужно много вариантов, чтобы быть действительно полезным. В этом случае вы можете либо отрефакторить класс или реализовывать статические конструкторы, которые скрывают реальные сложные конструкторы: например. Шниц.NewBaz (foo, bar) может фактически вызвать реальный конструктор, передающий правильные параметры.

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

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

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

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

было бы действительно странно иметь объект, который требует 7 или более параметров, чтобы дать объекту полное состояние и все это действительно является внешним по своей природе.

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

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

см. ниже :

public class Toto {
    private final String state0;
    private final String state1;
    private final String state2;
    private final String state3;

    public Toto(String arg0, String arg1, String arg2, String arg3) {
        this.state0 = arg0;
        this.state1 = arg1;
        this.state2 = arg2;
        this.state3 = arg3;
    }

    public static class TotoBuilder {
        private String arg0;
        private String arg1;
        private String arg2;
        private String arg3;

        public TotoBuilder addArg0(String arg) {
            this.arg0 = arg;
            return this;
        }
        public TotoBuilder addArg1(String arg) {
            this.arg1 = arg;
            return this;
        }
        public TotoBuilder addArg2(String arg) {
            this.arg2 = arg;
            return this;
        }
        public TotoBuilder addArg3(String arg) {
            this.arg3 = arg;
            return this;
        }

        public Toto newInstance() {
            // maybe add some validation ...
            return new Toto(this.arg0, this.arg1, this.arg2, this.arg3);
        }
    }

    public static void main(String[] args) {
        Toto toto = new TotoBuilder()
            .addArg0("0")
            .addArg1("1")
            .addArg2("2")
            .addArg3("3")
            .newInstance();
    }

}