-
Notifications
You must be signed in to change notification settings - Fork 0
Code smell
Park EunYoung edited this page Jan 16, 2019
·
6 revisions
- 한 곳 이상에서 중복된 코드 구조가 나타난다면, 그것을 합쳐서 프로그램을 개선할 수 있다.
- Extract Method
- 메소드에 파라미터와 임시변수가 많다면 Extract Method드 하기 힘듬.
- 최대한 파라미터와 임시변수를 제거하고 그것도 힘들다면 Object를 생성하자
- 클래스 하나가 많은 일을 하려 할 때, 지나치게 많은 인스턴스 변수가 나타난다.
- 이는 중복된 코드가 존재할 확률이 높다.
- 거대하다는 건 많은 책임을 가지고 있다는 의미.
- 클래스 내에서 서로에게 의미가 있는 변수를 골라서 묶는 작업을 하자.
- 긴 파라미터 리스트는 이해하기 어렵다.
- 다른 데이터가 필요할 떄마다 고쳐야함.
- 하나의 클래스가 다른 이유로 인해 다른 방법으로 자주 변경되는 경우
- 하나의 클래스가 여러 책임을 가지고 있기 때문임.
- 변경을 할 때마다 많은 클래스를 조금씩 수정해야 하는 경우.
- 변경해야 할 부분을 하나의 클래스로 몰자.
- 메소드가 자신이 속한 클래스보다 다른 클래스에 관심을 가지는 경우
- 어떤 값을 계산하기 위해 get 메소드를 호출하는 경우, 위임하는 것이 적절하다고 볼 수 있다.
- 필드 리스트와 파라미터 리스트를 줄이는 것은 확실히 나쁜 냄새를 제거할 것이지만, 객체를 갖게 되면 좋은 향기를 만들 수 있는 기회를 얻게 되는 것이다.
- 항상 몰려다니는 그룹이 있다면 클래스로 추출하자
- 다형성(polymorphism) switch 중복을 쉽게 찾아 바꾸어 줄 수 있는 좋은 도구다.
- switch문을 볼 때면 항상 다형성을 생각해야 한다.
- 만약 하나의 메소드에만 영향을 미치는 몇 개의 경우가 있다면, 굳이 바꿀 필요가 없다. 이런 경우 다형성은 과하다.
- 평행 상속 구조는 실제로 산탄총 수술의 특별한 경우이다.
- 한 클래스의 서브클래스를 만들면, 다른 곳에도 모두 서브클래스를 만들어 주어야 한다.
- 한쪽 상속 구조에서 클래스 이름의 접두어와 같은 경우에 이 냄새를 인식할 수 있다.
- 클래스를 생성할 때마다 그것을 유지하고, 이해하기 위한 비용이 발생한다.
- 이 비용을 감당할만큼 충분한 일을 하지 않는 클래스는 삭제되어야 한다.
- 거의 필요없는 클래스에 대해서는 Inline Class를 적용해야 한다.
- 만약 별로 하는 일이 없는 추상 클래스가 있으면 상속관계를 없애라.
- 불필요한 위임(delegation)은 Inline Class로 제거될 수 있다.
- 메소드에 사용되지 않는 파라미터는 지우고. 메소드 이름이 이상하고 추상적일 때는 구체적인 이름으로 바꾸자.
- 사용되지 않는 것처럼 보이는 변수가 왜 있는지를 이해하려고 하는 것은 매우 짜증나는 일이다.
- 복잡한 알고리즘이 여러 변수를 필요로 할 때 흔히 나타난다. 왜냐하면 아주 많은 값을 파라미터로 넘기는 것을 원치 않기 때문.
- 이러한 경우 필요한 변수와 메소드를 묶어 하나의 클래스로 추출할 수 있다.
- 클라이언트가 어떤 객체를 얻기 위해 다른 객체에 물어보고 다른 객체는 또 다시 다른 객체에 물어보고 그 객체는 다시 다른 객체에 물어보고 … 이런 경우 메시지 체인을 볼 수 있다.
- 이것은 긴 줄의 getThis 메소드 또는 임시변수의 시퀀스로 볼 수 있다. 이런 식으로 진행된다는 것은 클라이언트가 클래스 구조와 결합되어 있다는 것을 뜻한다. 중간에 어떤 관계가 변한다면 클라이언트 코드도 변경되어야 한다.
- 이 경우 Hide Delegate를 사용할 수 있다. 체인의 여러 지점에서 이것을 적용할 수 있다.
- 원칙적으로는 체인 내의 모든 객체에 적용할 수 있지만, 이 경우 종종 중간에 있는 모든 객체를 미들 맨(middle man)으로 만드는 결과를 초래할 수 있다.
- 그것을 사용하는 코드의 조각을 취해 Extract Method를 사용할 수 있는지 보고 Move Method로 그것을 체인의 밑으로 밀어넣는다.
- 객체의 주요 특징하나가 캡슐화(encapsulation - 내부의 상세사항을 외부로부터 숨기는 것)이다. 캡슐화는 보통 위임(delegation)과 함께 사용된다.
- 클래스의 인터페이스를 보니 메소드의 태반이 다른 클래스로 위임하고 있다면 Remove Middle Man을 사용해여 그 객체에 실제로 뭐가 어떻게 되어가고 있는지 알게 해줄 때이다.
- 몇몇 메소드가 많은 일을 하지 않는다면 Inline Method를 사용하여 호출하는 곳에 코드를 삽입할 수 있다.
- 추가 동작이 있다면 Replace Delegation with Inheritance을 사용하여 미들맨을 실제 객체의 서브클래스로 바꿀 수도 있다.
- 게이트웨이 역할만 하는 클래스(함수)
- 부적절한 친밀
- 높은 의존도 혹은 필요없는 분리
- 클래스가 지나치게 친밀하게 되어 서로 사적인 부분을 파고드느라 너무 많은 시간을 소모할 수 있다.
- 조각으로 나누고, 친밀함을 줄여야 한다. 공통 관심사가 있다면 공통된 부분을 안전한 곳으로 빼내서 하나의 클래스로 만들어라.
- 상속은 종종 과도한 친밀을 유도할 수 있다. 서브클래스는 항상 그 부모가 알려주고 싶은 것보다 많은 것을 알려고 한다.
##다른 인터페이스를 가진 대체 클래스(Alternative Classes with Different Interface)
- 같은 작업을 하지만 다른 시그니처(signature)를 가지는 메소드에 대해서는 Rename Method를 사용하라.
- 부족하다고 느끼면 클래스가 여전히 충분한 작업을 하지 않은 경우이다.
- 프로토콜이 같아질 때까지 Move Method를 이용하여 동작을 이동시켜라.
- 재사용은 종종 객체의 목적으로 선전된다. 우리는 재사용이 과대평가 되었다고 생각한다.
- 클래스 라이브러리를 만드는 사람이라고 모든 것을 다 아는 것은 아니다.
- 라이브러리가 종종 나쁜 형태이고, 원하는 것을 하기 위해 라이브러리 클래스를 수정하는 것은 거의 불가능하다는 것.
- 필드와 각 필드에 대한 get/set 메소드만 가지고, 다른 것은 아무것도 없는 클래스도 있다.
- 사용하고 있는 객체의 행위를 데이터 쪽으로 메소드를 추가한다.
- 데이터 클래스는 아이들과 같다. 처음 시작할 때는 괜찮지만, 어른 객체로 참여하기 위해서는 약간의 책임을 가질 필요가 있다.
- 서브클래스는 메소드와 데이터를 그 부모클래스로부터 상속 받는다 만약 서브클래스가 그들에게 주어진 것을 원하지 않는다거나 필요하지 않는다면 어떻게 될까? 전통적인 관점에서 이것은 클래스 상속 구조가 잘못 되었다는 것을 뜻한다.
- 원하지 않는 부분이 있다면 서브클래스로 위임을 해야하며 부모 클래스는 공통적인 부분만 가지고 있어야 한다.
- 주석을 써야 할 것 같은 생각이 들면, 먼저 코드를 리팩토링 하여 주석이 불필요하도록 하라.