한줄짜리 Extract Method

2013-04-24 23:09


if(this.fooService.checkCompleted(list)){ ... this.barService.updateStatus(item,Status.READY) }

한줄짜리 Extract Method 하시나요?? 하신다면 이유는 무엇인가요?? 반대하신다면 어떻이유때문에 반대하시나요???

1개의 의견 from FB

5개의 의견 from SLiPP

2013-04-25 10:14

먼저 위 코드에서 개선했으면 하는 부분은 this을 써야 되는가이다. 상태 정보를 포함하는 속성의 인자로 전달되는 값과 비교하기 위해 this를 사용하는 것은 이해가 되나 barService와 같이 클래스의 의존관계를 처리하는 부분까지 this를 사용할 필요는 없지 않을까 생각한다. 뭐 이게 취향이라면 할 수 없는 거고...

개인적으로는 한줄까지 Extract Method까지 극단적으로 리팩토링하지는 않는다. 특히 위 코드와 같이 조건문에서 한 가지 경우만 확인하는 경우에는 더 리팩토링하지 않는다. 왜냐하면 해당 조건 부분에 대부분 의도를 드러내는 경우가 많기 때문이다.

하지만 한 가지 조건이라도 의도가 들어나지 않거나 두 개 이상의 조건(and나 or로 연결되는)이 발생하는 경우는 한줄이라도 Extract Method 리팩토링을 하려고 노력한다. 이 기준을 판단하는 것이 쉽지는 않고 귀찮은 작업이기는 하지만 마음의 여유가 있는 경우에는 하게 되더라. 나름 재미도 있고... 소스 코드의 가독성 측면에서 의도를 들어내기 위해 하는 것도 좋은 습관이라 생각한다.

2013-04-25 11:51

위에 올려주신 코드가 extract 수행 전 or 후 인지 궁금합니다. 현 상태가 extract 후라고 하면 크게 건드리지는 않겠습니다. 전 이라고 해도 크게..

다만, 코드 사이에 있는 "..." 이게 뭐냐? 저 if block 전 후 무엇을 풀고 있나?(코드 레벨)에 따라.. if block이 extract 대상으로 검토가 가능 할 듯 합니다.

2013-04-25 17:19

제 Extract method의 기준은, 주석작성이 더 이상 필요없는가? 입니다. 즉, 주석없이 method 이름만으로도 어떤 일을 하는지 나타낼 수 있는 수준까지 쪼개는거죠~ (단순하고도.. 교과서적인 방식이네요~)

method가 어떤 일을 하는 지 명확해졌다면 그 자체를 원자단위로 보고 더 이상은 손대지 않습니다 ^^;;

2013-04-26 22:28

@양완수 Clean Code나 Smalltalk Best Practice Patterns를 보면 코드의 가치에 대해 이야기합니다. 지금의 extract method는 그 가치 중 단순성과 커뮤니케이션의 trade off라고 보여집니다.

메서드를 추가한다면 단순성을 저해하지만 커뮤니케이션을 강화시켜줄 가능성이 있죠. 아래는 버튼이 깜빡여야 한다의 구현입니다.

color.reverse(); 

extract method 하면 아래와 같습니다.

void highlight() {
    color.reverse();
}

이제 원글의 문맥을 적용해보죠. 원본은 아래와 같아요.

barService.updateStatus(item,Status.READY);

extract method하면 아래와 같죠. 코드의 커뮤니케이션 능력이 복잡도 비용에 비해 개선되었나요?

void updateStatusToReady() {
    barService.updateStatus(item,Status.READY)
}

저는 메서드를 추가해서 복잡도를 높인 비용이 좀 아깝네요.

의견 추가하기

연관태그

← 목록으로