매개 변수가 너무 많은 (6 개 이상) 메서드를 리팩터링하는 가장 좋은 방법은 무엇입니까?
때때로 불편한 매개 변수 수가있는 메소드를 발견합니다. 종종 그들은 생성자 인 것 같습니다. 더 나은 방법이 있어야 할 것 같지만 그것이 무엇인지 모르겠습니다.
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);
그래서 그것은 개선 된 것 같지 않습니다. 그렇다면 최선의 접근 방식은 무엇입니까?
가장 좋은 방법은 인수를 함께 그룹화하는 방법을 찾는 것입니다. 이것은 여러 인수 "그룹화"로 끝날 경우에만 실제로 작동합니다.
예를 들어 직사각형에 대한 사양을 전달하는 경우 x, y, 너비 및 높이를 전달하거나 x, y, 너비 및 높이를 포함하는 직사각형 객체를 전달할 수 있습니다.
리팩토링 할 때 이와 같은 것을 찾아서 정리하십시오. 논쟁이 정말로 결합 될 수 없다면, 단일 책임 원칙을 위반했는지 살펴보기 시작하십시오.
나는 당신이 C # 을 의미한다고 가정 할 것 입니다. 이 중 일부는 다른 언어에도 적용됩니다.
몇 가지 옵션이 있습니다.
생성자에서 속성 설정 자로 전환합니다 . 이것은 어떤 값이 어떤 매개 변수에 해당하는지 독자에게 분명하기 때문에 코드를 더 읽기 쉽게 만들 수 있습니다. 객체 이니셜 라이저 구문은이를 멋지게 만듭니다. 자동 생성 된 속성을 사용하고 생성자 작성을 건너 뛸 수 있으므로 구현도 간단합니다.
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
클래스를 사용하면 생성 된 코드를 수정하지 않고도 기본 클래스와 빌더를 모두 확장 할 수 있습니다.
"Introduce Parameter Object"리팩토링 . 리팩토링 카탈로그를 참조하십시오 . 아이디어는 전달하는 매개 변수 중 일부를 가져 와서 새 유형에 넣은 다음 대신 해당 유형의 인스턴스를 전달하는 것입니다. 생각하지 않고이 작업을 수행하면 시작했던 위치로 돌아갑니다.
new C(a, b, c, d);
된다
new C(new D(a, b, c, d));
그러나이 접근 방식은 코드에 긍정적 인 영향을 미칠 수있는 가장 큰 잠재력을 가지고 있습니다. 따라서 다음 단계를 따라 계속하십시오.
함께 의미가있는 매개 변수의 하위 집합 을 찾습니다 . 함수의 모든 매개 변수를 무의식적으로 그룹화하는 것만으로는별로 도움이되지 않습니다. 목표는 의미있는 그룹을 만드는 것입니다. 새로운 유형의 이름이 분명 할 때 올바른 것을 알 수 있습니다.
이러한 값이 함께 사용되는 다른 위치를 찾고 거기에서도 새 유형을 사용하십시오. 이미 모든 곳에서 사용하는 일련의 값에 대해 좋은 새 유형을 찾았을 때 그 새 유형이 모든 곳에서도 의미가있을 것입니다.
기존 코드에 있지만 새 유형에 속하는 기능을 찾으십시오.
예를 들어 다음과 같은 코드가 표시 될 수 있습니다.
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>
.)
보시다시피 Introduce Parameter Object는 좋은 시작 이었지만 실제 가치는 모델에서 누락 된 유용한 유형을 발견하는 데 도움이되었다는 것입니다.
생성자 인 경우, 특히 오버로드 된 변형이 여러 개있는 경우 빌더 패턴을 살펴 봐야합니다.
Foo foo = new Foo()
.configBar(anything)
.configBaz(something, somethingElse)
// and so on
정상적인 방법이라면 전달되는 값 간의 관계에 대해 생각하고 전송 객체를 만들어야합니다.
이것은 Fowler와 Beck의 책 "리팩토링"에서 인용되었습니다.
긴 매개 변수 목록
프로그래밍 초기에 우리는 루틴에 필요한 모든 것을 매개 변수로 전달하도록 배웠습니다. 대안은 글로벌 데이터이고 글로벌 데이터는 악하고 일반적으로 고통스럽기 때문에 이해할 수 있습니다. 필요한 것이 없으면 언제든지 다른 물건에게 가져다달라고 요청할 수 있기 때문에 물건은이 상황을 바꿉니다. 따라서 객체를 사용하면 메서드에 필요한 모든 것을 전달하지 않습니다. 대신 메서드가 필요한 모든 것을 얻을 수 있도록 충분히 전달합니다. 메서드에 필요한 많은 것은 메서드의 호스트 클래스에서 사용할 수 있습니다. 객체 지향 프로그램에서 매개 변수 목록은 기존 프로그램보다 훨씬 작은 경향이 있습니다. 긴 매개 변수 목록은 이해하기 어렵고, 일관성이 없어지고 사용하기 어렵고, 더 많은 데이터가 필요할 때 영원히 변경하기 때문에 좋습니다. 새 데이터를 가져 오기 위해 몇 번만 요청하면되기 때문에 객체를 전달하면 대부분의 변경 사항이 제거됩니다. 이미 알고있는 객체를 요청하여 하나의 매개 변수에서 데이터를 가져올 수있는 경우 매개 변수를 메소드로 바꾸기를 사용하십시오. 이 개체는 필드이거나 다른 매개 변수 일 수 있습니다. 전체 개체 유지를 사용하여 개체에서 수집 한 데이터를 가져와 개체 자체로 바꿉니다. 논리 개체가없는 데이터 항목이 여러 개있는 경우 매개 변수 개체 소개를 사용합니다. 이러한 변경에는 한 가지 중요한 예외가 있습니다. 이것은 호출 된 개체에서 더 큰 개체로의 종속성을 명시 적으로 만들지 않으려는 경우입니다. 이러한 경우 데이터의 압축을 풀고 매개 변수로 보내는 것이 합리적이지만 관련된 고통에주의를 기울이십시오.
이에 대한 고전적인 대답은 클래스를 사용하여 매개 변수의 일부 또는 전부를 캡슐화하는 것입니다. 이론 상으로는 훌륭하게 들리지만 저는 영역에서 의미가있는 개념에 대한 클래스를 만드는 사람이기 때문에이 조언을 적용하는 것이 항상 쉬운 것은 아닙니다.
예 : 대신 :
driver.connect(host, user, pass)
당신은 사용할 수 있습니다
config = new Configuration()
config.setHost(host)
config.setUser(user)
config.setPass(pass)
driver.connect(config)
YMMV
나는 현명한 균열처럼 들리고 싶지 않지만 전달하는 데이터가 실제로 전달되어야 하는지 확인 해야합니다. 생성자 (또는 해당 문제에 대한 메서드)에 물건을 전달하는 것은 약간의 냄새가납니다. 물체 의 동작 에 거의 중점을 두지 않습니다 .
오해하지 마세요 : 메서드와 생성자 는 때때로 많은 매개 변수를 가질 것 입니다. 그러나 발생하면 대신 행동으로 데이터 를 캡슐화하는 것을 고려하십시오 .
이런 종류의 냄새 (우리가 리팩토링에 대해 이야기하고 있기 때문에이 끔찍한 단어가 적절 해 보입니다 ...)는 속성이나 getter / setter가 많은 객체에 대해서도 감지 될 수 있습니다.
긴 매개 변수 목록을 볼 때 첫 번째 질문은이 함수 또는 객체가 너무 많이 수행하고 있는지 여부입니다. 치다:
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);
나는 걱정된다. 전화를 보면 이러한 모든 비밀 번호, 코드 및 플래그가 의미하는 바가 전혀 명확하지 않습니다. 이것은 단지 오류를 요구하는 것입니다. 프로그래머는 매개 변수의 순서에 대해 쉽게 혼란스러워하고 실수로 두 개를 전환 할 수 있으며, 동일한 데이터 유형 인 경우 컴파일러는이를 수락합니다. 필자는이 모든 것이 열거 형인 서명을 선호하므로 호출은 "A"대신 Type.ACTIVE 및 "false"대신 CreditWatch.NO 등으로 전달됩니다.
생성자 매개 변수 중 일부가 선택 사항 인 경우 생성자에서 필수 매개 변수를 가져오고 빌더를 리턴하는 선택 사항에 대한 메소드가 다음과 같이 사용되는 빌더를 사용하는 것이 좋습니다.
return new Shniz.Builder(foo, bar).baz(baz).quux(quux).build();
이에 대한 자세한 내용은 Effective Java, 2nd Ed., p. 11. 메서드 인수의 경우 동일한 책 (189 페이지)에서 매개 변수 목록을 줄이는 세 가지 방법을 설명합니다.
- 메서드를 더 적은 인수를 사용하는 여러 메서드로 나눕니다.
- 매개 변수 그룹을 나타 내기 위해 정적 도우미 멤버 클래스를 만듭니다. 즉, and
DinoDonkey
대신 a 를 전달합니다.dino
donkey
- 매개 변수가 선택 사항 인 경우 위의 빌더를 메소드에 채택하여 모든 매개 변수에 대한 객체를 정의하고 필요한 매개 변수를 설정 한 다음 실행 메소드를 호출 할 수 있습니다.
기본 생성자와 속성 설정자를 사용합니다. 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) { ...
This is a far different case because all the objects are different (and aren't likely to be muddled up). Agreed that if all objects are necessary, and they're all different, it makes little sense to create a parameter class.
Additionally, are some parameters optional? Are there method override's (same method name, but different method signatures?) These sorts of details all matter as to what the best answer is.
* A property bag can be useful as well, but not specifically better given that there is no background given.
As you can see, there is more than 1 correct answer to this question. Take your pick.
You can try to group your parameter into multiples meaningful struct/class (if possible).
I would generally lean towards the structs approach - presumably the majority of these parameters are related in some way and represent the state of some element that is relevant to your method.
If the set of parameters can't be made into a meaningful object, that's probably a sign that Shniz
is doing too much, and the refactoring should involve breaking the method down into separate concerns.
If your language supports it, use named parameters and make as many optional (with reasonable defaults) as possible.
I think the method you described is the way to go. When I find a method with a lot of parameters and/or one that is likely to need more in the future, I usually create a ShnizParams object to pass through, like you describe.
How about not setting it in all at once at the constructors but doing it via properties/setters? I have seen some .NET classes that utilize this approach such as Process
class:
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();
You can trade complexity for source code lines. If the method itself does too much (Swiss knife) try to halve its tasks by creating another method. If the method is simple only it needs too many parameters then the so called parameter objects are the way to go.
I concur with the approach of moving the parameters into a parameter object (struct). Rather than just sticking them all in one object though, review if other functions use similar groups of parameters. A paramater object is more valuable if its used with multiple functions where you expect that set of parameters to change consistently across those functions. It may be that you only put some of the parameters into the new parameter object.
If you have that many parameters, chances are that the method is doing too much, so address this first by splitting the method into several smaller methods. If you still have too many parameters after this try grouping the arguments or turning some of the parameters into instance members.
Prefer small classes/methods over large. Remember the single responsibility principle.
Named arguments are a good option (presuming a language which supports them) for disambiguating long (or even short!) parameter lists while also allowing (in the case of constructors) the class's properties to be immutable without imposing a requirement for allowing it to exist in a partially-constructed state.
The other option I would look for in doing this sort of refactor would be groups of related parameters which might be better handled as an independent object. Using the Rectangle class from an earlier answer as an example, the constructor which takes parameters for x, y, height, and width could factor x and y out into a Point object, allowing you to pass three parameters to the Rectangle's constructor. Or go a little further and make it two parameters (UpperLeftPoint, LowerRightPoint), but that would be a more radical refactoring.
It depends on what kind of arguments you have, but if they are a lot of boolean values/options maybe you could use a Flag Enum?
I think that problem is deeply tied to the domain of the problem you're trying to solve with the class.
In some cases, a 7-parameter constructor may indicate a bad class hierarchy: in that case, the helper struct/class suggested above is usually a good approach, but then you also tend to end up with loads of structs which are just property bags and don't do anything useful. The 8-argument constructor might also indicate that your class is too generic / too all-purpose so it needs a lot of options to be really useful. In that case you can either refactor the class or implement static constructors that hide the real complex constructors: eg. Shniz.NewBaz (foo, bar) could actually call the real constructor passing the right parameters.
One consideration is which of the values would be read-only once the object is created?
Publicly writable properties could perhaps be assigned after construction.
Where ultimately do the values come from? Perhaps some values are truely external where as others are really from some configuration or global data that is maintained by the library.
In this case you could conceal the constructor from external use and provide a Create function for it. The create function takes the truely external values and constructs the object, then uses accessors only avaiable to the library to complete the creation of the object.
It would be really strange to have an object that requires 7 or more parameters to give the object a complete state and all truely being external in nature.
When a clas has a constructor that takes too many arguments, it is usually a sign that it has too many responsibilities. It can probably be broken into separate classes that cooperate to give the same functionalities.
In case you really need that many arguments to a constructor, the Builder pattern can help you. The goal is to still pass all the arguments to the constructor, so its state is initialized from the start and you can still make the class immutable if needed.
See below :
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();
}
}
'programing' 카테고리의 다른 글
C auto 키워드는 어디에 사용됩니까? (0) | 2020.09.02 |
---|---|
스프링 부트에서 휴식을 위해 기본 URL을 설정하는 방법은 무엇입니까? (0) | 2020.09.02 |
응답 상태 코드 설정 (0) | 2020.09.02 |
버튼 클릭 Android에서 소리 재생 (0) | 2020.09.02 |
div의 맨 위로 스크롤 (0) | 2020.09.02 |