# 개요

리팩토링 9장, 조건문 간결화 읽고 간단 정리

각 카탈로그에서 설명하는 내용 중 중요하게 생각하는 부분 작성하고 내 생각도 덧붙임

코드 작성하다가 이 예시가 이거구나 싶을 때는 예시코드도 붙일 예정

 


# 조건문 간결화

객체 지향 프로그램은 절차 프로그램에 비해 조건에 따른 동작이 대체로 적은데, 이것은 조건에 따른 동작 대부분이 재정의를 통해 처리되기 때문이다.

재정의 방식이 좋은 이유는 호출 코드가 조건문 원리를 알 필요가 없어 조건문을 확장하기가 더 간편하기 때문이다.

 


Decompose Conditional (조건문 쪼개기)

복잡한 조건문이 있을 땐 각 부분을 메서드로 빼내자

조건을 검사하고 다양한 조건에 따라 다른 작업을 처리하는 코드를 작성하다 보면 금방 메서드가 길어진다. 메서드는 길어지기만 해도 알아보기 힘든데 조건문까지 많으면 더 심각하다.

  • 6장의 메서드 추출 카탈로그에서 한 얘기와 비슷하다. 결국 메서드는 짧고 이해하기 쉬워야 하는데 조건문이 길면 이해하기 어렵다. 바로 적절한 이름을 가진 메서드로 추출하자.

 


Consolidate Conditional Expression (중복 조건식 통합)

여러 조건 검사식의 결과가 같을 땐 하나의 조건문으로 합친 후 메서드로 빼내자

조건문을 합쳐야 하는 이유는 두 가지이다.

  1. 조건식을 합치면 여러 검사를 OR 연산자로 연결해서 하나의 검사 수행을 표현해서 무엇을 검사하는지 확실히 이해할 수 있다.
  2. 이러한 조건식 통합을 실시하면 메서드 추출을 할 기반이 마련된다.

조건식이 독립적이고 하나의 검사로 인식되지 말아야 할 땐 이 기법을 사용하면 안된다. 그 코드는 이미 개발자의 의도에 맞기 때문이다.

  • 나는 대체로 isValid같은 메서드를 만들 때 이런 고민을 많이한다. ||나 && 연산자를 이용하여 한 줄로 간단하게 표현하는게 좋을까? 아니면 각 하나의 조건이 valid/invalid를 명시하도록 하기 위해 조건절을 나누는게 좋을까?
  • 근데 대체로 조건절을 나눠서 표현하는 경우가 많다. (그게 맞는지는 정확히 모르겠다)

 


Consolidate Duplicate Conditional Fragments (조건식의 공통 실행 코드 빼내기)

조건문의 모든 절에 같은 코드가 있을 땐 같은 부분을 조건문 밖으로 빼내자

  • 조건문이 늘어나면 코드가 당연히 길어지는데 최대한 중복을 줄일 수 있는 부분은 줄이는 것이 중요하다고 생각한다.

 


Remove Control Flag (제어 플래그 제거)

논리 연산식의 제어 플래그 역할을 하는 변수가 있을 땐 그 변수를 break나 return문으로 바꾸자

제어 플래그는 유용함을 능가하는 단점이 있다. 제어 플래그로 인해 이탈점이 한 개가 되면 코드 안의 각종 특이한 플래그로 조건문이 복잡해진다.

  • 플래그가 하나면 그러려니 하겠는데 여러 개인 경우 정말 복잡해진다. 거기다 break/continue/return 등으로 해당 조건이 탈출 조건이라는 것을 명시할 수 있어서 가독성 면에서도 더 좋다고 생각한다.

 


Replace Nested Conditional with Guard Claueses (여러 겹의 조건문을 감시 절로 전환)

메서드에 조건문이 있어서 정상적인 실행 경로를 파악하기 힘들 땐 모든 특수한 경우에 감시 절을 사용하자.

조건문은 두 가지 형태를 띤다

  1. 어느 한 경로가 정상적인 동작의 일부인지 검사
  2. 조건식 판별의 한 결과만 정상적인 동작을 나타내고 나머지는 비정상적인 동작을 나타냄

조건문은 다양한 의도가 있는데 그 의도는 코드에 반영되어 드러나야 한다.

  1. 둘 다 정상 동작의 일부라면 if ~ else로 구성된 조건문을 사용하고
  2. 조건문이 특이한 조건이라며 그 조건을 검사하여 조건이 true일 경우 반환하도록 한다.

 


Replace Conditional with Polymorphism (조건문을 재정의로 전환)

객체 타입에 따라 다른 기능을 실행하는 조건문이 있을 땐 조건문의 각 절을 하위 클래스의 재정의 메서드 안으로 옮기고 원본 메서드는 abstract 타입으로 수정하자.

재정의의 본질은 타입에 따라 기능이 달라지는 여러 객체가 있을 때 일일이 조건문을 작성하지 않아도 다형적으로 호출되게 할 수 있다는 것이다.

  • 내 개인적인 생각인데 객체 지향 언어의 꽃이 이 부분이라고 생각한다. 이 기법을 잘 사용하면 코드 확장성도 유용해지고 하나의 클래스는 하나의 역할만 갖게 된다고 생각한다.

 


# 내 생각

사실상 조건문을 재정의로 전환하는것이 가장 중요하고 그만큼 어렵다고 생각한다. 이번에 프로젝트를 할 때 책을 보며 이 기법을 적용 해 보았는데 확실히 코드가 가독성도 좋아졌고 새 기능을 추가하기도 쉬워졌다는 생각이 들었다.

 

 

# 개요

리팩토링 8장, 데이터 체계화 읽고 간단 정리

각 카탈로그에서 설명하는 내용 중 중요하게 생각하는 부분 작성하고 내 생각도 덧붙임

코드 작성하다가 이 예시가 이거구나 싶을 때는 예시코드도 붙일 예정


# 데이터 체계화

객체지향 언어는 구형 언어의 간순 데이터 타입으론 불가능했던 것까지 할 수 있는 새로운 타입을 정의 할 수 있어서 좋다.


Self Encapsulate Field (필드 자체 캡슐화)

필드에 직접 접근할 때 그 필드로의 결합에 문제가 생길 때 getter/setter 메서드를 작성하여 그 메서드를 통해서 접근하게 하자

  • VO, DTO등의 클래스를 만들면 습관적으로 getter/setter 메서드를 만들긴 한다. (lombok을 쓰기도)
  • 근데 사실 습관적으로 getter/setter 메서드를 만드는 경향이 있다.
  • 캡슐화 목적으로 필드에 직접 접근할 수 없게 private로 선언하고 메서드를 통해서 접근하도록 하기 위해서이다.
  • 하지만 그게 캡슐화일까? 결국 직접 접근이 안 될 뿐이지 메서드로 접근할 수 있는데 그게 정보보호/은닉의 효과가 있다고 생각은 하지 않는다. 확실하게 아는 것이 아니기 때문에 더 공부해야 할 것 같다.

변수 간접 접근 방식은 하위 클래스가 메서드에 해당 정보를 가져오는 방식을 재정의 할 수 있어서 데이터 관리가 더 유연해진다.

변수 직접 접근 방식은 코드를 더욱 알아보기 쉽게 한다.

  • 클린코드의 관점으로 봤을 땐 더욱 알아보기 쉬운 직접 접근 방식이 더 좋은것 같다.

 


Replace Data Value with Object (데이터 값을 객체로 전환)

데이터 항목에 데이터나 기능을 더 추가해야 할 때는 데이터 항목을 객체로 만들자.

개발 초기 단계에는 단순 정보를 간단한 데이터 항목으로 표현하는데 개발이 진행되다 보면 그런 간단한 항목이 점점 복잡해진다.

한 두 항목은 객체 안에 메서드를 넣어도 되겠지만 금세 '중복 코드'나 '잘못된 소속'이라는 구린내가 풍기게 된다. 그렇다면 즉시 데이터 값을 객체로 전환하자.

  • 주로 한 클래스의 한 메서드 내에 코드를 계속 작성하다 보면 어느 부분을 메서드로 추출해야하는 때가 생긴다.
  • 그렇게 메서드 추출을 통해 메서드를 여러개 만들다 보면 어떤 변수는 모든 메서드에서 공통으로 사용되기 때문에 필드로 올려야 할 때가 생긴다.
  • 또 그렇게 계속 작성하다보면 한 클래스에 여러 필드가 생기게 되고 그 필드들을 하나의 객체로 전환하여 사용하는 것이 낫다는 얘기인 것 같다.

 


Change Value to Reference (값을 참조로 변환)

클래스에 같은 인스턴스가 많이 들어 있어서 이것들을 하나의 객체로 바꿔야 할 땐 그 객체를 참조 객체로 전환하자

참조 객체는 고객이나 계좌 같은 것이다. 각 객체는 현실에서의 한 객체에 대응하므로 둘이 같은지 검사할 때 객체 ID를 사용한다.

값 객체는 날짜나 돈 같은 것이다. 전적으로 데이터 값을 통해서만 정의된다. 두 객체가 같은지 판단할 땐 equals과 hashCode 메서드를 재정의 해야 한다.

  • 값 객체와 참조 객체가 이해는 되는데 막상 코드 짜다보면 크게 구분 없이 작성하게 되는 것 같다. 두 용어가 자주 헷갈리기도 하고..

 


Change Reference to Value (참조를 값으로 전환)

참조 객체가 작고 수정할 수 없고 관리하기 힘들 땐 그 참조 객체를 값 객체로 만들자

참조 객체를 사용한 작업이 복잡해지는 순간이 참조를 값으로 바꿔야 할 시점이다. 참조 객체는 어떤 식으로든 제어되어야 한다.

값 객체는 변경할 수 없어야 한다는 주요 특성이 있다. 하나에 대한 질의를 호출하면 항상 결과가 같아야 한다.

  • Money라는 클래스가 있고 value가 125라고 한다면 A가 가진 Money와 B가 가진 Money 객체는 같아야 한다. 반면 Person이라는 클래스의 name이 A도 아무무고 B도 아무무라고 같은 Person은 아니다.
  • 근데 이게 단적인 예로 보면 이해가 잘 되는데 막상 코드 작성할 땐 크게 생각 안하고 작성한다.. 구별하는게 큰 의미가 있는건지도 잘 모르겠다.

 


Replace Array with Object (배열을 객체로 전환)

배열을 구성하는 특정 원소가 별의 별 의미를 지닐 땐 그 배열을 각 원소마다 필드가 하나씩 든 객체로 전환하자.

배열은 비슷한 객체들의 컬렉션을 일정 순서로 담는 용도로만 사용해야 한다.

 


Duplicate Observed Data (관측 데이터 복제)

도메인 데이터는 GUI 컨트롤 안에서만 사용 가능한데, 도메인 메서드가 그 데이터에 접근해야 할 땐 그 데이터를 도메인 객체로 복사하고 양측의 데이터를 동기화 하는 관측 인터페이스 Observer를 작성하자.

비즈니스 로직과 UI가 분리되는 이유

  1. 비슷한 비즈니스 로직을 여러 인터페이스가 처리 해야 하는 경우라서
  2. 비즈시스 로직까지 처리하려면 UI가 너무 복잡해져서
  3. GUI와 분리된 도메인 객체가 더욱 유지보수 하기 쉬워서
  4. 두 부분을 서로 다른 개발자가 다루게 될 수 있어서

비즈니스 로직과 UI를 분리할 때 기능은 간단히 분리할 수 있지만 데이터는 분리하기 어려울 때가 많다. 도메인 모델에 있는 데이터와 같은 의미를 지닌 데이터를 GUI 컨트롤에 넣어야하기 때문이다.

MVC를 시작으로 사용자 인터페이스 프레임워크는 이러한 데이터를 제공하고 모든 데이터의 동기화를 유지하는 다층 시스템을 사용했다.

  • MVC 패턴의 예로 비유하자면 View에서 비즈니스 로직을 처리하면 안된다는 얘기인것 같다.
  • 내가 했던 프로젝트를 생각하면 데이터를 복사하진 않고 View에서 Controller의 메서드를 호출하게 하고 Controller에서 비즈니스 로직을 수행 후 결과 객체를 View로 반환했는데, 이 방법을 얘기하는게 맞는건지 잘 모르겠다..

 


Change Unidirectional Association to Bidirectional (클래스의 단방향 연결을 양방향으로 전환)

두 클래스가 서로의 기능을 사용해야 하는데 한 방향으로만 연결되어 있을 땐 역 포인터를 추가하고 두 클래스를 모두 업데이트 할 수 있게 접근 한정자를 수정하자

역방향 참조가 아닌 다른 경로를 찾아서 해결 할 수도 있다. 하지만 이게 불가능 할 때는 양방향 참조를 설정해야한다. 처음엔 뒤죽박죽되기 쉽지만 익숙해지면 별로 복잡하지 않다.

  • MVC를 예로 생각하면 View에서 Controller를 필드로 갖고 마찬가지로 Controller에서도 View를 필드로 갖는 형태인 것 같은데, 결합도가 강해져서 별로 좋은 선택은 아닌것 같다.
  • 그리고 처음엔 뒤죽박죽되기 쉽지만 익숙해지면 별로 복잡하지 않다는 얘기 자체가 뒤죽박죽 될 수 있다는 얘기인데 굳이 그런 위험을 감수해야할까?

 


Change Bidirectional Association to Unidirectional (클래스의 양방향 연결을 단방향으로 전환)

두 클래스가 양방향으로 연결되어 있는데 한 클래스가 다른 클래스의 기능을 더 이상 사용하지 않게 됐을 땐 불필요한 방향의 연결을 끊자

양방향 연결은 쓸모가 많지만 대가가 따른다. 양방향 연결을 유지하고 객체가 적절히 생성되고 제거되는지 확인하는 복잡함이 더해진다.

양방향 연결로 인해 두 클래스는 서로 종속된다. 한 클래스를 수정하면 다른 클래스도 변경된다. 종속성이 많으면 시스템의 결합력이 강해져서 사소한 수정에도 얘기치 못한 각종 문제가 발생한다.

  • 위에서 얘기했듯이 이 이유 때문에 굳이 위험을 감수해서 양방향 연결을 하는것은 좋지 않다고 생각한다. 다른 방법으로 해결하는 것이 더 낫다고 생각한다.

 


Replace Magic Number with Symbolic Constant (매직 넘버를 상수로 전환)

특수 의미로 지닌 리터럴이 있을 땐 의미를 살린 이름의 상수를 작성한 후 리터럴을 상수로 교체하자

상수를 사용하면 단점이나 부작용 없이 성능이 향상되며 가독성이 엄청나게 향상된다

  • 이 기법도 중요하면서도 간단하게 할 수 있는 리팩토링이라고 생각한다.

 


Encapsulate Collection (컬렉션 캡슐화)

메서드가 컬렉션을 반환할 땐 그 메서드가 읽기 전용 뷰를 반환하게 수정하고 추가 메서드와 삭제 메서드를 작성하자.

읽기 메서드는 컬렉션 객체 자체를 반환해서는 안된다. 컬렉션을 참조하는 부분이 반환 받은 컬렉션의 내용을 조작해도 그 컬렉션이 든 클래스는 무슨 일이 일어나는지 알 수 없기 때문이다. 

이로 인해 컬렉션을 참조하는 코드에게 데이터의 구조가 지나치게 노출된다. 값이 여러 개인 속성을 읽는 읽기 메서드는 컬렉션 조작이 불가능한 형식을 반환하고 불필요하게 자세한 컬렉션 구조 정보는 감춰야 한다.

  • 내가 했던 프로젝트를 예로 들면 getter가 List를 반환 할 때 그냥 List 필드 자체를 반환했는데 필드 List에 들어있는 데이터들을 새 List에 복사해서 반환하는것이 더 안전하다는 생각이 들었다.

 


Replace Type Code with Class(분류 부호를 클래스로 전환)

기능에 영향을 미치는 숫자형 분류 부호가 든 클래스가 있을 땐 그 숫자들을 새 클래스로 바꾸자

분류 부호 이름을 상징적인 것으로 정하면 코드가 상당히 이해하기 쉬워진다.

 


Replace Type Code with Subclass (분류 부호를 하위 클래스로 전환)

클래스 기능에 영향을 주는 변경 불가 분류 부호가 있을 땐 분류 부호를 하위 클래스로 만들자.

분류 부호가 클래스 기능에 영향을 미치는 현상은 조건문이 있을 때 주로 나타난다.

분류 부호의 값을 검사해서 그 값에 따라 다른 코드를 실행하는 경우이다. 이런 조건문은 재정의로 바꿔야한다.

재정의를 하기 위해서 상속 구조로 고쳐야 하는데 가장 간단한 방법이 분류 부호를 하위 클래스로 전환하는 것이다.

이 기법의 장점은 클래스 사용 부분에 있던 다형적인 기능 관련 데이터가 클래스 자체로 이동 된다는 것이다.

변형된 새 기능을 추가할 땐 하위 클래스만 하나 더 추가하면 된다. 재정의를 이용하지 않는다면 조건문을 전부 찾아서 일일이 수정 해야 한다.

 


Replace Type Code with State/Strategy (분류 부호를 상태/전략 패턴으로 전환)

분류 부호가 클래스의 기능에 영향을 주지만 하위 클래스로 전환할 수 없을 땐 그 분류 부호를 상태 객체로 만들자

하나의 알고리즘을 단순화 해야 할 때는 전략 패턴이 더 적절하고, 강태별 데이터를 이동하고 객체를 변화하는 상태로 생각할 때는 상태 패턴이 더 적절하다

 


# 내 생각

대부분은 잘 이해가 됐지만 어려웠던 부분이 몇 개 있었다. 

1. 값 ↔ 참조로 전환

  • 값과 참조 자체는 이해가 설명과 예시 코드를 보니 이해가 되긴 했는데,  막상 내가 코드를 작성할 때 두 개를 크게 구분 지어서 생각하지 않았던 것 같다.

2. 관측 데이터 복제

  • Ovserver 패턴을 잘 모르기도 했고, MVC 패턴을 알고는 있지만 데이터를 동기화 한다기 보다는 단순히 Controller에서 View로 값을 반환하는 형태로 이해하고 있어서 내용과 조금 다른 것 같다.

3. 분류 부호 → 클래스/하위클래스/상태or패턴 전환

  • 미니 프로젝트를 하며 나름대로 적용 해 보았지만 내가 완전히 이해하고 딱 딱 적용했다기 보다는 책을 보며 겨우 해낸 리팩토링이라 더 공부해야 할 것 같다. 근데 확실히 그렇게 리팩토링 해 놓으니 확장성이 좋아졌다는 느낌을 받았다.

# 개요

리팩토링 7장, 객체 간의 기능 이동 읽고 간단 정리

각 카탈로그에서 설명하는 내용 중 중요하게 생각하는 부분 작성하고 내 생각도 덧붙임

코드 작성하다가 이 예시가 이거구나 싶을 때는 예시코드도 붙일 예정

 

# 객체 간의 기능 이동

객체 설계에서 가장 중요한 일 중 하나가 '기능을 어디에 넣을지 판단'하는것이다.

  • 20년간 작업해 왔는데도 아직 서툴다고한다. 개 구라같지만 그만큼 어렵다는 뜻이겠지

클래스가 방대해지는 원인은 대체로 기능이 너무 많기 때문이다. 이럴 때는 클래스 추출을 통해 많은 기능을 분리 해야 한다.

  • 확실하진 않지만 OOP 원칙중 SRP에 해당하는 내용인것 같다.
  • 메서드와 마찬가지로 클래스도 작아야한다고 생각한다. 메서드가 점점 길어질 때 메서드를 분리해야하는 것처럼 한 클래스에 기능이 점점 늘어나면 클래스도 분리해야한다.

 

## Move Method (메서드 이동)

한 클래스에 기능이 너무 많거나 클래스가 다른 클래스와 과하게 연동되어 의존성이 지나칠 때는 메서드를 옮기는 것이 좋다.

  • 객체간의 의존도를 줄이는 일은 항상 어렵다. 작은 프로젝트에서도 어려운데 규모가 커지면 더 어렵겠지..
  • 한 클래스는 하나의 책임을 가져야 한다는 말이 아직 잘 와닿진 않지만 클래스 이름에 무언가 맞지 않는 기능을 하는 메서드가 있다면, 그 메서드는 그 클래스에 있어야 할 메서드가 아니라는 생각을 한다.

 

## Move Field (필드 이동)

지금은 합리적이고 올바르다고 판단되는 설계라도 나중에는 그렇지 않을 수 있다. 문제는 그게 아니라 그러한 상황 변화에 아무런 대처도 하지 않는 것이다.

  • 내가 한 대부분의 프로젝트들은 한 번 하고 치워버리는 성향이 강했다. (과제나 그냥 작은 토이프로젝트..) 설계도 간단하고 설계가 변하는 일도 없었다. 과제같은 경우는 오히려 설계는 교수나 조교들이 다 했지 ㅋㅋ..
  • 설계가 항상 변할 수 있다는 생각도 중요하고 어느 상황에나 유연하게 대처할 능력이 필요하다고 생각한다.

 

## Extract Class (클래스 추출)

클래스는 확실하게 추상화 되어야 하며, 두 세가지 명확한 기능을 담담해야 한다.

  • 내가 이해하는 추상화는 그 클래스명, 메서드명만 보고도 이 모듈이 어떤 기능을 하는지 이해할 수 있도록 코드를 작성하는 것이다.
  • 그만큼 메서드명, 클래스명을 잘 짓는것이 중요하고, 클래스에 맞는 메서드들만 가지고 있어야한다고 생각한다. 그러면 내부 구현이 어떤 로직인지 자세히 알 필요없다. 이 모듈이 뭘 하는지 이름만 보고도 아니깐

데이터의 일부분과 메서드의 일부분이 한 덩어리이거나 주로 함께 변화하거나 서로 유난히 의존적인 데이터의 일부분도 클래스로 떼어내기 좋다.

 

## Inline Class (클래스 내용 직접 삽입)

주로 클래스의 기능 대부분을 다른 곳으로 옮기는 리팩토링을 수행하고 남은 기능이 거의 없어졌을 때 나타난다.

 

## Hide Delegate (대리 객체 은폐)

캡슐화란 객체가 시스템의 다른 부분에 대한 정보의 일부분만 알 수 있게 은폐하는 것을 뜻한다. 객체를 캡슐화하면 무언가를 변경할 때 그 변화를 전달해야할 객체가 줄어듦으로 변경하기 쉬워진다.

클라이언트가 서버 객체의 필드 중 하나에 정의된 메서드를 호출할 때 클라이언트는 이 대리 객체에 관해 알아야 한다. 대리 객체가 변경될 때 클라이언트도 변경해야 할 가능성이 있기 때문이다. 이런 의존성을 없애려면 대리 객체를 감추는 간단한 위임 메서드를 서버에 두면 된다. 변경은 서버에만 이뤄지고 클라이언트에는 영향을 주지 않는다.

  • 음 쉽게 생각하면 a객체 내에 있는 b객체의 메서드를 쓰기위해서 a.getB().somethingMetodOfB()이런 식으로 열거해서 쓰는것 보다. a 내의somethingMethod()에서 b객체의 메서드를 사용하는것이 좋다는 얘기 같다.

 

## Remove Middle Man (과잉 중개 메서드 제거)

대리 객체 사용을 캡슐화하면 얻는 장점도 있지만 단점도 생긴다. 클라이언트가 대리 객체의 새 기능을 사용해야 할 때마다 서버에 간단한 위임 메서드를 추가해야 한다는 점이다.

은폐의 적절한 정도를 알기란 어렵다. 하지만 리팩토링을 실시할 때는 은폐의 정도를 잘 몰라도 된다. 시스템이 변경되면 은폐의 정도의 기준도 변한다. 전에는 적절했던 캡슐화가 현재 시점에서는 부적절할 수도 있다. 리팩토링은 필요해질 때마다 보수하면 된다.

  • 리팩토링 하고 후회하지 말라고 하는것 같다. 어짜피 다시 롤백하면 되니까
  • 근데 말처럼 안된다 리팩토링하고 "이게 맞나? 전의 코드가 더 낫지않나?" 라는 고민을 할 때가 자주 있다.

 

## 내 생각

메서드 내에서만 해결하면 됐던 리팩토링에서 클래스로 옮겨가면 조금씩 어려워진다. 이게 클래스가 적으면 그러려니 하겠는데 프로젝트에서 클래스가 적을리가..

여기에 이제 디자인 패턴, OOP 원칙 등 더 머리 아파지는게 나오면 머리에 과부하오는데 리팩토링을 이해하기 위해서 선행 학습이 어느정도 필요한것 같다 ㅜ

# 개요

리팩토링 6장, 메서드 정리 부분 읽고 간단 정리

각 카탈로그에서 설명하는 내용 중 중요하게 생각하는 부분 작성하고 내 생각도 덧붙임

코드 작성하다가 이 예시가 이거구나 싶을 때는 예시코드도 붙일 예정

 

# 메서드 정리

리팩토링의 주된 작업은 코드를 포장하는 메서드를 적절히 정리하는 것이다.

  • 매우 동의한다. 메서드가 길어지면 읽기도, 이해하기도 힘들어진다. 메서드는 짧아야하고 그 메서드가 하는 기능이 메서드 이름으로 명확하게 드러나야 한다고 생각한다.
  • 메서드가 점점 길어진다는 것은 메서드가 하나의 기능이 아니라 여러 기능을 하게 된다는 뜻이다. 그러므로 메서드 추출을 통해 추상화하자.

메서드 추출에서 가장 힘든 작업은 지역변수를 처리하는 것인데, 그건 주로 임시변수 때문이다.

  • 나는 잘 와닿지 않는게, 요즘 IDE에서 리팩토링 기능을 많이 지원해준다. 그래서 내가 메서드 추출하고 싶은 부분을 드래그해서 몇번 '딸깍'하면 어느 변수를 매개변수로 보낼지, 어느 변수에 반환값을 대입해줘야 하는지 잘 지원해준다.
  • 물론 변수가 많아지면 IDE가 제공해주는 기능을 쓰기 어려울 때도 있다. 더 좋은 방법이 있을 수 있겠지만, 나는 메서드 내의 지역변수들을 전부 클래스 필드로 올려버린다. 그 이후에 메서드 추출과정에서 지역변수로 보내거나 쿼리 메서드로 바꾸는 작업을 한다.

직관적인 이름의 간결한 메서드가 좋은 이유

  1. 메서드가 적절히 잘게 쪼개져 있으면 다른 메서드에서 쉽게 사용할 수 있다.
  2. 상위 계층의 메서드에서 주석같은 더 많은 정보를 읽어들일 수 있다.
  3. 재정의 하기도 훨씬 수월하다.
  • 1번에 "다른 메서드에서 쉽게 사용할 수 있다" 라는 내용을 보고 생각난 건데, 옛날에는 꼭 다른 여러 메서드에서 사용하기 위해 메서드 추출을 해야한다고 생각했다. 그래서 "하나의 메서드에서만 사용하는데 굳이 메서드 추출할 필요가 있을까?" 라는 생각도 했었는데, 메서드의 호출 횟수에 따라 메서드를 추출 해야한다는 것은 엉터리같은 생각이었다.

중요한 것은 메서드의 길이가 아니라 메서드 이름과 메서드 내용의 의미적 차이라고 할 수 있다.

  • 맞는 표현인지 모르겠는데 추상화 레벨이 같아야한다는 의미같다.
  • 예전에 유튜브에서 백명석의 클린 코더스 영상에서 비슷한 내용을 들은것 같다.. 나중에 참고해서 내용추가하자..

리팩토링의 핵심은 의도한 기능을 하눈ㄴ에 파악할 수 있는 직관적인 메서드명을 사용하는 것과 메서드를 간결하게 만드는 것이다.

 

## Extract Method (메서드 추출)

메서드 추출 기법은 제일 많이 사용된다.

  • 가장 많이 사용되고, 가장 쉽게 할 수 있는 리팩토링 중 하나라고 생각한다.

메서드가 너무 길거나 코드에 주석을 달아야만 의도를 이해할 수 있을 때 별도의 메서드를 만든다.

  • "이 부분이 무슨 기능을 한다" 라고 주석을 달지 말고 메서드로 추출하자.

 

## Inline Method (메서드 대신 내용 직접 삽입)

간혹 메서드명에 모든 기능이 반영될 정도로 메서드 기능이 지나치게 단순할 때가 있다.

  • 나는 보통 조건문 작성할 때 이런 상황이 많이 생기는데, 자주 고민한다. 보통은 메서드로 추출하는 편이긴한데 그러고 또 한참 고민한다.. 너무 어렵..

 

## Inline Temp (임시 변수 내용 직접 삽입) && Replace Temp with Query (임시변수를 메서드 호출로 전환)

임시변수를 메서드 호출로 수정하면 클래스 안의 모든 메서드가 그 정보에 접근할 수 있다. 이렇게 하면 클래스의 코드가 훨씬 깔끔해진다.

  • 코드 작성할 땐 임시변수에 어떤 값이 저장되어 있는지 확인하기 위해 임시변수에 값을 대입해놓는다. 제대로 동작하는 것을 확인 한 후에는 임시변수를 지워버리고 메서드 호출로 대체한다.

 

## Introduce Explaining Variable (직관적 임시변수 사용)

수식이 너무 복잡해져서 이해하기 힘들 수 있다. 이럴 때는 임시변수를 사용하면 더 처리하기 쉽게 쪼갤 수 있다.

 

## Split Temporary Variable (임시변수 분리)

한 임시 변수에 여러 값을 대입하지 마라

  • 이건 너무 당연한 얘기라.. 예를 들어 임시변수 하나 생성해서 둘레, 면적을 같이 대입하면 안되고, 둘레를 저장하는 임시변수와 면적을 저장하는 임시변수 따로 선언해야한다는 얘기이다.

 

## Replace Method with Method Object (메서드를 메서드 객체로 전환)

지역변수 때문에 메서드 추출을 적용할 수 없는 긴 메서드가 있을 때 그 메서드 자체를 클래스로 만들어서 모든 지역변수를 클래스의 필드로 만들고 클래스 내의 여러 메서드로 쪼갠다.

  • IDE 도움을 받기 힘들 때 내가 하는 방법과 비슷한것 같다.

 

## 내 생각

몇몇 기법들은 카탈로그 제목만 보고 "이게 뭐지?" 라는 생각이었는데 예제를 보니까 내가 의식하지 않아도 당연하게 하고 있었던 기법들이었다. 당연한 만큼 간결하고 깔끔한 코드 작성을 위해서 이 챕터의 내용들이 기본이 되어야 한다고 생각한다.

이전 포스팅

 

2021.12.15 - [Refactoring] - [Refactoring] 중복 코드 제거1

 

[Refactoring] 중복 코드 제거1

2021.12.09 - [Etc] - Refactoring 일지 1. APIView To ModelViewSet Refactoring 일지 1. APIView To ModelViewSet APIView → ModelViewSet 사용하기 프로젝트를 시작할 때, 나는 기존에 자바를 썻기 때문에, 파..

jino-dev-diary.tistory.com

  • build_data() 메소드는 어느정도 리팩토링이 된 것 같다. (아직 부족하지만)

이번에 할 리팩토링

  • NoteViewSet, TopicViewSet, PageViewSet 상속받기
  • 각 ViewSet(NoteViewSet, TopicViewSet, PageViewSet)의 create() 메소드가 매우 비슷한 로직을 가지고있다. 즉, 중복 코드가 발생한다.
  • 또한 TopicViewSet과 PageViewSet의 list() 메소드도 로직이 매우 유사하다.
  • 이것을 저번 포스팅에서 build_data() 메소드를 BaseData 라는 상위 클래스를 만들어 NoteData, TopicData, PageData 클래스가 상속받도록 한 것처럼 BaseContentViewSet이라는 상위 클래스를 만들어 NoteViewSet, TopicViewSet, PageViewSet 클래스가 상속받도록 해서 BaseContentViewSet에 공통 메서드를 구현하면 되지 않을까? 라는 생각으로 바로 리팩토링을 시작했다.
  • 다형성을 이용하는 리팩토링은 여전히 익숙하지 않다. 내가 아직 초보라고 느끼고, 그나마 잘 할 수 있는 리팩토링 수준은 메소드추출, 인라인코드, 매직넘버 to 상수..? 이정도의 간단한 수준이기 때문에 다형성을 이용한 리팩토링은 좀 오래 걸릴것 같다고 겁(?)을 조금 먹었다.
  • 그럼에도 불구하고 나에게는 테스트코드가 있다. 이것 저것 시도해보고 잘 안되면 원복하고 "아, 이 방법은 좀 더 공부가 필요하거나, 여기서는 적용하기 힘들겠다;"하고 블로그나 노트에 정리만 잘 해놓으면 된다. (비록 그 삽질동안 잡아먹는 시간은 아까울수 있지만..)
  • 바로 리팩토링을 시작하기 전에 대충 구조를 상상했는데 다음과 같다.

계정 인증을 위한 CtrlfAuthenticationMixinNoteViewSet, TopicViewSet, PageViewSet 모두 필요하니까 ModelViewSetCtrlfAuthenticationMixin 클래스를 상속받은 BaseContentViewSet을 구현하고, NoteViewSet, TopicViewSet, PageViewSetBaseContentViewSet을 상속받도록 하고, create() 메소드는 셋 다 로직이 같다. (각 Model과 Issue에 대한 data를 만들고, serializer를 통해 검증 후, Model과 Issue 레코드를 생성한다) Model과 Issue에 대한 data를 만드는 로직만 Model마다 조금 다를 뿐이다. 그러므로 BaseContentViewSetcreate() 메소드를 공통 메소드로 만들고 각 하위 클래스에서 BaseContentViewSetcreate() 메소드를 호출하면 중복을 확실히 줄일 수 있을 것 같다.

이런 생각을 가지고 리팩토링을 바로 시작했다.

리팩토링 시작

1. NoteViewSet, TopicViewSet, PageViewSet 클래스의 상위 클래스인 BaseContentViewSet 생성

더보기
class NoteViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    ...

    @swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        note_data, issue_data = NoteData().build_data(request, ctrlf_user)

        note_serializer = NoteSerializer(data=note_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)

        if note_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=note_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)

     ...


class TopicViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    ...

    @swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        topic_data, issue_data = TopicData().build_data(request, ctrlf_user)

        topic_serializer = TopicSerializer(data=topic_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)

        if topic_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=topic_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)
        return Response(status=status.HTTP_201_CREATED)

    ...


class PageViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    ...

    @swagger_auto_schema(**SWAGGER_PAGE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        page_data, issue_data = PageData().build_data(request, ctrlf_user)

        page_serializer = PageSerializer(data=page_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)

        if page_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=page_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)
     ...
  • 각 Viewset(NoteViewSet, TopicViewSet, PageViewSet)의 create() 메소드의 로직이 전부 비슷하다.
    • 각 model별로 data가 조금씩 다른 것 뿐, 그 외에는 로직이 전부 같다.
  • Upper 클래스를 생성하여 나머지 ViewSet이 상속받도록 변경해보자

 

더보기
class BaseContentViewSet(ModelViewSet):
    ...

    def create(self, request, *args, **kwargs):
        related_model_serializer = self.get_serializer(data=kwargs["model_data"])
        issue_serializer = IssueCreateSerializer(data=kwargs["issue_data"])

        if related_model_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=related_model_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)

class NoteViewSet(CtrlfAuthenticationMixin, BaseContentViewSet):
    serializer_class = NoteSerializer

    ...

    @swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
                ctrlf_user = self._ctrlf_authentication(request)
        note_data, issue_data = NoteData().build_data(request)
        data = {"model_data":note_data, "issue_data":issue_data}

        return super().create(request, **data)


class TopicViewSet(CtrlfAuthenticationMixin, BaseContentViewSet):
    serializer_class = TopicSerializer

    ...

    @swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        topic_data, issue_data = TopicData().build_data(request)
        data = {"model_data":topic_data, "issue_data":issue_data}

        return super().create(request, **data)


class PageViewSet(CtrlfAuthenticationMixin, BaseContentViewSet):
    serializer_class = PageSerializer
    ...

    @swagger_auto_schema(**SWAGGER_PAGE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        page_data, issue_data = PageData().build_data(request)
        data = {"model_data":page_data, "issue_data":issue_data}

        return super().create(request, **data)
  • Upper 클래스인 BaseContentViewSet을 만들어 중복코드를 줄였다.
  • 근데 ctrlf_user 의 로직도 중복코드라서 BaseContentViewSet으로 옮기고 싶다

 

2. basedata.py 수정

더보기
class BaseData:
    def __init__(self):
        pass

    def build_data(self, request, ctrlf_user):
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "action": CtrlfActionType.CREATE,
        }
        return model_data, issue_data


class NoteData(BaseData):
    def __init__(self):
        super().__init__()

    def build_data(self, request, ctrlf_user):
        note_data, issue_data = super(NoteData, self).build_data(request, ctrlf_user)
        issue_data["related_model_type"] = CtrlfContentType.NOTE

        return note_data, issue_data


class TopicData(BaseData):
    def __init__(self):
        super().__init__()

    def build_data(self, request, ctrlf_user):
        topic_data, issue_data = super(TopicData, self).build_data(request, ctrlf_user)
        topic_data["note"] = request.data["note_id"]
        issue_data["related_model_type"] = CtrlfContentType.TOPIC

        return topic_data, issue_data


class PageData(BaseData):
    def __init__(self):
        super().__init__()

    def build_data(self, request, ctrlf_user):
        page_data, issue_data = super(PageData, self).build_data(request, ctrlf_user)
        page_data["topic"] = request.data["topic_id"]
        page_data["content"] = request.data["content"]
        issue_data["related_model_type"] = CtrlfContentType.PAGE
        return page_data, issue_data
  • ctrlf_user 처리를 BaseContentViewSet에서 하기 위해 basedata.py에서 삭제하자.

 

더보기
# basedata.py
from rest_framework.request import Request
from .models import CtrlfActionType, CtrlfContentType, CtrlfIssueStatus

class BaseData:
    request: Request

    def __init__(self, request):
        self.request = request

    def build_data(self):
        model_data = {
            "title": self.request.data["title"],
        }
        issue_data = {
            "title": self.request.data["title"],
            "reason": self.request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "action": CtrlfActionType.CREATE,
        }
        return model_data, issue_data


class NoteData(BaseData):
    def build_data(self):
        note_data, issue_data = super().build_data()
        issue_data["related_model_type"] = CtrlfContentType.NOTE

        return {"model_data": note_data, "issue_data": issue_data}


class TopicData(BaseData):
    def build_data(self):
        topic_data, issue_data = super().build_data()
        topic_data["note"] = self.request.data["note_id"]
        issue_data["related_model_type"] = CtrlfContentType.TOPIC

        return {"model_data": topic_data, "issue_data": issue_data}


class PageData(BaseData):
    def build_data(self):
        page_data, issue_data = super().build_data()
        page_data["topic"] = self.request.data["topic_id"]
        page_data["content"] = self.request.data["content"]
        issue_data["related_model_type"] = CtrlfContentType.PAGE

        return {"model_data": page_data, "issue_data": issue_data}
  • ModelViewSetcreate() 메소드에서 request를 인자로 받도록 했다.
  • 이에 따라 ModelViewSetcreate() 메소드도 조금 수정했다.

 

3. create() 메소드 수정

더보기
class BaseContentViewSet(ModelViewSet):
    ...

    def create(self, request, *args, **kwargs):
        related_model_serializer = self.get_serializer(data=kwargs["model_data"])
        issue_serializer = IssueCreateSerializer(data=kwargs["issue_data"])

        if related_model_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=related_model_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)

class NoteViewSet(CtrlfAuthenticationMixin, BaseContentViewSet):
    serializer_class = NoteSerializer

    ...

    @swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
                ctrlf_user = self._ctrlf_authentication(request)
        note_data, issue_data = NoteData().build_data(request)
        data = {"model_data":note_data, "issue_data":issue_data}

        return super().create(request, **data)


class TopicViewSet(CtrlfAuthenticationMixin, BaseContentViewSet):
    serializer_class = TopicSerializer

    ...

    @swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        topic_data, issue_data = TopicData().build_data(request)
        data = {"model_data":topic_data, "issue_data":issue_data}

        return super().create(request, **data)


class PageViewSet(CtrlfAuthenticationMixin, BaseContentViewSet):
    serializer_class = PageSerializer
    ...

    @swagger_auto_schema(**SWAGGER_PAGE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        page_data, issue_data = PageData().build_data(request)
        data = {"model_data":page_data, "issue_data":issue_data}

        return super().create(request, **data)
  • basedata.pybuild_data() 메소드를 수정함에 따라 각 ModelViewSet에서 data를 구성하는 로직도 수정하고, ctrlf_user 로직 부분도 BaseContentViewSet으로 옮기자.

 

더보기
class BaseContentViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    ...

    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        related_model_serializer = self.get_serializer(data=kwargs["model_data"])
        issue_serializer = IssueCreateSerializer(data=kwargs["issue_data"])

        if related_model_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=related_model_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)

class NoteViewSet(BaseContentViewSet):
    serializer_class = NoteSerializer

    ...

    @swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        data = NoteData(request).build_data()
        return super().create(request, **data)


class TopicViewSet(BaseContentViewSet):
    serializer_class = TopicSerializer

    ...

    @swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        data = TopicData(request).build_data()
        return super().create(request, **data)


class PageViewSet(BaseContentViewSet):
    serializer_class = PageSerializer
    ...

    @swagger_auto_schema(**SWAGGER_PAGE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        data = PageData(request).build_data()
        return super().create(request, **data)

 

4. if else statement 삭제

더보기
class BaseContentViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    ...

    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        kwargs["model_data"]["owners"] = [ctrlf_user.id]
        kwargs["issue_data"]["owner"] = ctrlf_user.id
        related_model_serializer = self.get_serializer(data=kwargs["model_data"])
        issue_serializer = IssueCreateSerializer(data=kwargs["issue_data"])

        if related_model_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=related_model_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)
  • if else를 굳이 사용할 필요가 없다. indent만 불편해지니까 삭제하자

 

더보기
class BaseContentViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    ...

    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        kwargs["model_data"]["owners"] = [ctrlf_user.id]
        kwargs["issue_data"]["owner"] = ctrlf_user.id
        related_model_serializer = self.get_serializer(data=kwargs["model_data"])
        issue_serializer = IssueCreateSerializer(data=kwargs["issue_data"])

        related_model_serializer.is_valid(raise_exception=True)
        issue_serializer.is_valid(raise_exception=True)
        issue_serializer.save(related_model=related_model_serializer.save())

        return Response(status=status.HTTP_201_CREATED)
  • is_valid() 메소드에서 exception이 발생할경우 400_BAD_REQUEST를 리턴하도록 exceptions.py에 구현되어있었기 때문에 여기서 굳이 구현할 필요가 없다.

 

커밋

후기

  • 커밋을 보면 create() 메소드의 로직을 BaseContentViewSet으로 옮기는 커밋에 list() 메소드의 리팩토링도 포함되어있다. 커밋 메시지를 더 자세하게 쓰던지, 커밋 단위를 자르던지 하는 연습이 필요할듯
  • 코드를 바로 작성하기전에 위 그림처럼 어느정도 머리속이나 낙서 수준으로 구조화 해놓고 구현하면 그렇게 하지 않을 때 보다 더 코드가 잘 짜지는것 같다. 좋은 습관일듯
  • 물론 내 테스트가 완벽하지는 않지만, 테스트가 있음으로 좀 더 과감하게 리팩토링 할 수 있다는걸 느꼈다. 하다가 실패하면 다시 이전 코드로 돌아가면 그만이다.

'Refactoring' 카테고리의 다른 글

[Refactoring] 중복 코드 제거1  (0) 2021.12.15
[Refactoring] APIView To ModelViewSet  (0) 2021.12.09

2021.12.09 - [Etc] - Refactoring 일지 1. APIView To ModelViewSet

 

Refactoring 일지 1. APIView To ModelViewSet

APIView → ModelViewSet 사용하기 프로젝트를 시작할 때, 나는 기존에 자바를 썻기 때문에, 파이썬도 처음이었고, django도 처음이었고 drf도 처음이었다. 그래서 다른 멘티들이나 멘토님 코드보고 따라

jino-dev-diary.tistory.com

APIView → ModelViewSet으로 변경하면서 이전보다 훨씬 코드가 깔끔해졌다. 하지만 아직 해결해야 하는 부분이 있었다.

 

note_data, topic_data, page_data, issue_data를 만드는 코드들이 중복이 매우 많다. 리팩토링 필요

note, topic, page create 메서드가 data가 조금 다른것을 제외하고는 역시 중복된 코드가 많다. 리팩토링 필요

topic, page list 메서드 또한 note_id, topic_id가 필요하다는 것만 다르고 중복된 코드다. 리팩토링 필요

DRF 공식문서를 보면, ModelViewSet.as_view()를 쓰는 것보다 Router를 이용하다는것이 보편적이라고 한다.(Typical) 공부할겸 읽어보고 수정이 필요하다면 수정할 계획

처리해야할 부분 중 가장 쉽게 할 수 있는것이 Serializing을 위해 note, topic, page, issue data를 만드는 코드들의 중복을 제거하는 것이라고 판단해서 바로 시작했다.

 

1. build_data() 메소드 추출

더보기
더보기
더보기
더보기
class NoteViewSet(CtrlfAutentication, ModelViewset):
	# list(), retrieve() 등 일부 코드 생략
	@swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
	def create(self, request, *args, **kwargs):
		ctrlf_user = self._ctrlf_authentication(request)
		note_data, issue_data = self.build_data(request, ctrlf_user)
		...
		return Response(status=status.HTTP_201_CREATED)

	def build_data(self, request, ctrlf_user):
		note_data = {
			"title": request.data["title"],
			"owners": [ctrlf_user.id],
		}
		issue_data = {
			"owner": ctrlf_user.id,
			"title": request.data["title"],
			"reason": request.data["reason"],
			"status": CtrlfIssueStatus.REQUESTED,
			"related_model_type": CtrlfContentType.NOTE,
			"action": CtrlfActionType.CREATE,
		}
		return note_data, issue_data

class TopicViewSet(CtrlfAutentication, ModelViewset):
	# list(), retrieve() 등 일부 코드 생략
	@swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
	def create(self, request, *args, **kwargs):
		ctrlf_user = self._ctrlf_authentication(request)
		topic_data, issue_data = self.build_data(request, ctrlf_user)
		...
		return Response(status=status.HTTP_201_CREATED)

	def build_data(self, request, ctrlf_user):
		topic_data = {
			"title": request.data["title"],
			"owners": [ctrlf_user.id],
			"note": request.data["note_id"]
		}
		issue_data = {
			"owner": ctrlf_user.id,
			"title": request.data["title"],
			"reason": request.data["reason"],
			"status": CtrlfIssueStatus.REQUESTED,
			"related_model_type": CtrlfContentType.TOPIC,
			"action": CtrlfActionType.CREATE,
		}
		return topic_data, issue_data

class PageViewSet(CtrlfAutentication, ModelViewset):
	# list(), retrieve() 등 일부 코드 생략
	@swagger_auto_schema(**SWAGGER_PAGE_CREATE_VIEW)
	def create(self, request, *args, **kwargs):
		ctrlf_user = self._ctrlf_authentication(request)
		page_data, issue_data = self.build_data(request, ctrlf_user)
		...
		return Response(status=status.HTTP_201_CREATED)

	def build_data(self, request, ctrlf_user):
		page_data = {
			"title": request.data["title"],
			"owners": [ctrlf_user.id],
			"topic": request.data["topic_id"],
			"content": request.data["content"],
		}
		issue_data = {
			"owner": ctrlf_user.id,
			"title": request.data["title"],
			"reason": request.data["reason"],
			"status": CtrlfIssueStatus.REQUESTED,
			"related_model_type": CtrlfContentType.PAGE,
			"action": CtrlfActionType.CREATE,
		}
		return page_data, issue_data
  • build_data() 메소드가 note, topic, page에 따라 조금씩 다르지만 중복되는 부분이 분명히 있다.
  • 이를 하나의 메소드로 추출하고, note, topic, page에 따라 data가 다르게 만들어지도록 분기처리하면 중복코드가 없어진다. 메소드를 추출하자

 

더보기
def build_data(request, ctrlf_user, model):
    if model == "note":
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id], }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.NOTE,
            "action": CtrlfActionType.CREATE,
        }
    elif model == "topic":
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
            "note": request.data["note_id"]
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.TOPIC,
            "action": CtrlfActionType.CREATE,
        }
    else:
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
            "topic": request.data["topic_id"],
            "content": request.data["content"],
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.PAGE,
            "action": CtrlfActionType.CREATE,
        }

    return model_data, issue_data
	
class NoteViewSet(CtrlfAutentication, ModelViewset):
	# build_data() 외 그대로
	...
		note_data, issue_data = self.build_data(request, ctrlf_user, "note")
		...
	...

class TopicViewSet(CtrlfAutentication, ModelViewset):
	# build_data() 외 그대로
	...
		topic_data, issue_data = self.build_data(request, ctrlf_user, "topic")
		...
	...


class PageViewSet(CtrlfAutentication, ModelViewset):
	# build_data() 외 그대로
	...
		page_data, issue_data = self.build_data(request, ctrlf_user, "page")
		...
	...
  • 각 ViewSet class내부의 build_data() 메소드를 밖으로 추출했다.
  • 각 모델마다 data가 다르므로 이는 우선 분기처리하고, 추가 인자로 note, topic, page를 받도록 했다.
  • 여전히 중복된 부분이 있고, 리팩토링이나 클린코드를 읽었을때 이런식으로 분기처리하는건 좋은 코드가 아니라고 했다.
  • Step By Step으로 진행할것이기 때문에 일단은 그대로 둔다.

2. build_data() 메소드의 if.. else → polymorphism

더보기
def build_data(request, ctrlf_user, model):
    if model == "note":
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id], }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.NOTE,
            "action": CtrlfActionType.CREATE,
        }
    elif model == "topic":
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
            "note": request.data["note_id"]
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.TOPIC,
            "action": CtrlfActionType.CREATE,
        }
    else:
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
            "topic": request.data["topic_id"],
            "content": request.data["content"],
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.PAGE,
            "action": CtrlfActionType.CREATE,
        }

    return model_data, issue_data
  • model_data의 title, owners는 note, topic, page 모두 공통된 data이다.
  • issue_data의 title, owner, reason, status, action은 note, topic, page 모두 공통된 data이다.
  • 다형성을 이용해서 이것을 리팩토링해보자

 

더보기
# ctrlfbe/basedata.py
class BaseData:
    def __init__(self):
        pass

    def build_data(self, request, ctrlf_user):
        model_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "action": CtrlfActionType.CREATE,
        }
        return model_data, issue_data

class NoteData(BaseData):
    def __init__(self):
        super().__init__()

    def build_data(self, request, ctrlf_user):
        note_data, issue_data = super(NoteData, self).build_data(request, ctrlf_user)
        issue_data["related_model_type"] = CtrlfContentType.NOTE

        return note_data, issue_data

class TopicData(BaseData):
    def __init__(self):
        super().__init__()

    def build_data(self, request, ctrlf_user):
        topic_data, issue_data = super(TopicData, self).build_data(request, ctrlf_user)
        topic_data["note"] = request.data["note_id"]
        issue_data["related_model_type"] = CtrlfContentType.TOPIC

        return topic_data, issue_data

class PageData(BaseData):
    def __init__(self):
        super().__init__()

    def build_data(self, request, ctrlf_user):
        page_data, issue_data = super(PageData, self).build_data(request, ctrlf_user)
        page_data["topic"] = request.data["topic_id"]
        page_data["content"] = request.data["content"]
        issue_data["related_model_type"] = CtrlfContentType.PAGE
        return page_data, issue_data

# ctrlfbe/views.py
class NoteViewSet(CtrlfAutentication, ModelViewset):
	# build_data() 외 그대로
	...
		note_data, issue_data = NoteData().build_data(request, ctrlf_user)
		...
	...

class TopicViewSet(CtrlfAutentication, ModelViewset):
	# build_data() 외 그대로
	...
		topic_data, issue_data = TopicData().build_data(request, ctrlf_user)
		...
	...

class PageViewSet(CtrlfAutentication, ModelViewset):
	# build_data() 외 그대로
	...
		page_data, issue_data = PageData().build_data(request, ctrlf_user)
		...
	...
  • 가장 상위 클래스인 BaseData에서 공통된 data인 model_data(title, owners)과 issue_data(title, reason, owner, status, action)을 추가하고 각 NoteData, TopicData, PageData에서 필요한 data를 추가하도록 했다.
  • 근데 다형성을 이용한 리팩토링이 숙련이 안되어있어서 굉장히 어색하다.. 그래도 일단 두자.

 

3. Commit

 

4. 후기(?)

  • 그 동안 코딩하면서 Polymorphism을 이용하는것에 깊은 관심도 없었고, 그냥 "상속 받고 오버라이딩 해서 쓴다" 정도로만 이해하고 있어서 다형성을 이용한 리팩토링 기법에 대해 잘 몰랐다.
  • 책, 구글링, 유튜브 영상 등 많이 보고 흉내라도 내보긴 했는데 아직 잘 와닿지 않는다.. 더 연습 해야할것 같다.

 

5. 참고자료

'Refactoring' 카테고리의 다른 글

[Refactoring] 중복 코드 제거2(상속)  (0) 2021.12.25
[Refactoring] APIView To ModelViewSet  (0) 2021.12.09

APIView → ModelViewSet 사용하기

  • 프로젝트를 시작할 때, 나는 기존에 자바를 썻기 때문에, 파이썬도 처음이었고, django도 처음이었고 drf도 처음이었다. 그래서 다른 멘티들이나 멘토님 코드보고 따라하는 수준이었다.
  • 코드를 보고 따라했을 때, 다들 APIView를 사용했고, GenericView, ViewSet같은 것들이 있는지도 몰랐다.
  • 조금씩 공식문서를 보고 따라하고 학습하면서, APIView외에 GenericView, ViewSet이 있다는 것을 알게 되었고, List, Create, Retreive, Update, Delete View 클래스를 하나씩 만드는것 보다. ViewSet을 사용하여 하나의 클래스로 각 action을 해결하는것이 더 낫겠다는 생각이 들어서 APIViewModelViewSet으로 리팩토링을 시작했다.
  • 멘토님께서 강조했던 부분이 리팩토링을 하나의 피처로 정해서 하지 말고, 각 피처 구현이 끝나면 항상 리팩토링을 하라고 하셨다.
    • 이번 리팩토링을 하면서 왜 그렇게 말씀하셨는지 확실히 느꼈다. 이게 계속 리팩토링 요소가 쌓이니까 너무 힘들더라..

1. BaseContentView 삭제

class BaseContentView(APIView):
    child_model: Optional[Model] = None
    many = False

    def get(self, request, *args, **kwargs):
        id_from_path_param = list(kwargs.values())[0]
        result = self.parent_model.objects.filter(id=id_from_path_param).first()
        class_name_lower = str(self.parent_model._meta).split(".")[1]

        if result is None:
            return Response(
                data={"message": ERR_NOT_FOUND_MSG_MAP.get(class_name_lower, ERR_UNEXPECTED)},
                status=status.HTTP_404_NOT_FOUND,
            )

        if self.child_model:
            result = self.child_model.objects.filter(**{class_name_lower: result})
        if self.many:
            serializer = self.serializer(result, many=True)
        else:
            serializer = self.serializer(result)

        return Response(data=serializer.data, status=status.HTTP_200_OK)
  • BaseContentView는 Note, Topic, Page Detail API의 중복코드와 Topic, Page List API의 중복코드를 줄이기 위해 멘토님이 작성한 클래스다.
  • ModelViewSet으로 리팩토링하기 위해 이 클래스는 삭제했다.

2. Note API View 리팩토링

1. Note APIView(NoteListCreateView, NoteDetailUpdateDeleteview)

class NoteListCreateView(CtrlfAuthenticationMixin, APIView):
    @swagger_auto_schema(**SWAGGER_NOTE_LIST_VIEW)
    def get(self, request):
        current_cursor = int(request.query_params["cursor"])
        notes = Note.objects.all()[current_cursor : current_cursor + MAX_PRINTABLE_NOTE_COUNT]
        serializer = NoteSerializer(notes, many=True)
        serialized_notes = serializer.data
        return Response(
            data={"next_cursor": current_cursor + len(serialized_notes), "notes": serialized_notes},
            status=status.HTTP_200_OK,
        )

    @swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
    def post(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        note_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.NOTE,
            "action": CtrlfActionType.CREATE,
        }
        note_serializer = NoteSerializer(data=note_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)

        if note_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=note_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)

class NoteDetailUpdateDeleteView(BaseContentView):
    parent_model = Note
    serializer = NoteSerializer

    @swagger_auto_schema(**SWAGGER_NOTE_DETAIL_VIEW)
    def get(self, request, *args, **kwargs):
        return super().get(request, *args, **kwargs)
  • Note Create, List를 한 클래스로, Note Detail, Update, Delete를 한 클래스로 구현했다.
  • 여기서 두 클래스를 ModelViewSet을 상속받아 한 클래스로 리팩토링하자.
class NoteViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    queryset = Note.objects.all()
    serializer_class = NoteSerializer
    pagination_class = NoteListPagination
    lookup_url_kwarg = "note_id"

    @swagger_auto_schema(**SWAGGER_NOTE_LIST_VIEW)
    def list(self, request, *args, **kwargs):
        return super().list(self, request, *args, **kwargs)

    @swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        note_data, issue_data = self.build_data(request, ctrlf_user)

        note_serializer = NoteSerializer(data=note_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)

        if note_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=note_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)

        return Response(status=status.HTTP_201_CREATED)

    @swagger_auto_schema(**SWAGGER_NOTE_DETAIL_VIEW)
    def retrieve(self, request, *args, **kwargs):
        return super().retrieve(self, request, *args, **kwargs)

    def build_data(self, request, ctrlf_user):
        note_data = {
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
        }
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.NOTE,
            "action": CtrlfActionType.CREATE,
        }

        return note_data, issue_data
  • 기존의 NoteListCreateView에서의 get() 메소드에서는 query_paramcurrent_cursor를 담아서 구현했는데, ModelViewSetpagination_class가 있었다. 이것을 이용하고 싶어서 공식문서를 보면서 직접 NoteListPagination을 구현했다.
  • note_data, issue_data 만드는 부분을 build_data()로 분리했다.

2. NoteListPagination (커밋링크)

# ctrlfbe/paginations.py
class NoteListPagination(CursorPagination):
    max_page_size = MAX_PRINTABLE_NOTE_COUNT

    def paginate_queryset(self, queryset, request, view=None):
        self.current_cursor = int(request.query_params["cursor"])
        notes = Note.objects.all()[self.current_cursor : self.current_cursor + MAX_PRINTABLE_NOTE_COUNT]
        return notes

    def get_paginated_response(self, data):
        return Response(data={"next_cursor": self.current_cursor + len(data), "notes": data})
  • 요구사항에서 Note Paging 스펙이 Cursor based였기 때문에 CursorPagination을 상속받았다.

3. url.py 수정

# ctrlfbe/note_urls.py
urlpatterns = [
    path("", NoteListCreateView.as_view(), name="note_list_create"),
    path("<int:note_id>/", NoteDetailUpdateDeleteView.as_view(), name="note_detail_update_delete"),
    path("<int:note_id>/topics/", TopicListView.as_view(), name="topic_list"),
]
  • NoteListCreateViewNoteDetailUpdateDeleteView를 지웠기 때문에 note_urls.py 에서 에러가 났다.
  • NoteViewSet에 맞게 수정하자.
# ctrlfbe/note_urls.py
urlpatterns = [
    path(
        "",
        NoteViewSet.as_view(
            {
                "get": "list",
                "post": "create",
            }
        ),
        name="note_list_create",
    ),
    path("<int:note_id>/topics/", TopicListView.as_view(), name="topic_list"),
    path("<int:note_id>/", NoteViewSet.as_view({"get": "retrieve"}), name="note_detail_update_delete"),
  • as_view() 메서드에 HTTP method와(GET, POST 등) NoteViewSet의 메소드를 매핑했다.
  • Routers를 쓰는 방식도 공식문서에 나와있는데 봐도 잘 모르겠어서 일단은 이렇게 구현했다. Router쓰는것이 더 깔끔하다면 수정 할 예정.
  • python code formatting 도구인 black에 의해서 code formatting이 자동으로 됐는데.. 내가 보기에는 좀 불편하다 ㅜ

3. Topic API View 리팩토링

1. Topic APIView ( TopicListView, TopicCreateView, TopicDetailUpdateDeleteView)

class TopicListView(BaseContentView):
    parent_model = Note
    child_model = Topic
    serializer = TopicSerializer
    many = True

    @swagger_auto_schema(**SWAGGER_TOPIC_LIST_VIEW)
    def get(self, request, *args, **kwargs):
        return super().get(request, *args, **kwargs)

class TopicCreateView(CtrlfAuthenticationMixin, APIView):
    @swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
    def post(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        topic_data = {
            "note": request.data["note_id"],
            "title": request.data["title"],
            "owners": [ctrlf_user.id],
        }

        topic_serializer = TopicSerializer(data=topic_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)
        if topic_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=topic_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)
        return Response(status=status.HTTP_201_CREATED)

class TopicDetailUpdateDeleteView(BaseContentView):
    parent_model = Topic
    serializer = TopicSerializer

    @swagger_auto_schema(**SWAGGER_TOPIC_DETAIL_VIEW)
    def get(self, request, *args, **kwargs):
        return super().get(request, *args, **kwargs)
  • NoteDetailUpdateDeleteView와 유사하게 BaseContentView를 상속받았다.
  • 세 클래스를 PageViewSet 클래스 하나로 리팩토링하자.
class TopicViewSet(CtrlfAuthenticationMixin, ModelViewSet):
    queryset = Topic.objects.all()
    serializer_class = TopicSerializer
    lookup_url_kwarg = "topic_id"

    @swagger_auto_schema(**SWAGGER_TOPIC_LIST_VIEW)
    def list(self, request, *args, **kwargs):
        note_id = list(kwargs.values())[0]
        note = Note.objects.filter(id=note_id).first()
        if note is None:
            return Response(
                data={"message": ERR_NOT_FOUND_MSG_MAP.get("note", ERR_UNEXPECTED)},
                status=status.HTTP_404_NOT_FOUND,
            )

        return super().list(request, *args, **kwargs)

    @swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
    def create(self, request, *args, **kwargs):
        ctrlf_user = self._ctrlf_authentication(request)
        topic_data, issue_data = self.build_data(request, ctrlf_user)

        topic_serializer = TopicSerializer(data=topic_data)
        issue_serializer = IssueCreateSerializer(data=issue_data)

        if topic_serializer.is_valid() and issue_serializer.is_valid():
            issue_serializer.save(related_model=topic_serializer.save())
        else:
            return Response(status=status.HTTP_400_BAD_REQUEST)
        return Response(status=status.HTTP_201_CREATED)

    @swagger_auto_schema(**SWAGGER_TOPIC_DETAIL_VIEW)
    def retrieve(self, request, *args, **kwargs):
        return super().retrieve(self, request, *args, **kwargs)

    def build_data(self, request, ctrlf_user):
        topic_data = {"title": request.data["title"], "owners": [ctrlf_user.id], "note": request.data["note_id"]}
        issue_data = {
            "owner": ctrlf_user.id,
            "title": request.data["title"],
            "reason": request.data["reason"],
            "status": CtrlfIssueStatus.REQUESTED,
            "related_model_type": CtrlfContentType.TOPIC,
            "action": CtrlfActionType.CREATE,
        }

        return topic_data, issue_data
  • NoteViewSet과 매유 유사하다.
  • list() 메소드에서
    • Note의 경우는 url이 {BASE_URL}/notes/ 이고, cursor based pagination이다.
    • Topic의 경우는 url이 {BASE_URL}/{note_id}/topics/ 이고, pagination이 따로 없다. paging 스펙이 따로 없었기 때문에, default값이다. default는 LimitOffsetPagination이다. (공식문서)
  • create() 메소드를 보면, NoteViewSetcreate() 메소드와 매우 유사하다. 중복된 코드라서 리팩토링이 필요할 것 같다.

2. url.py 수정

# ctrlfbe/note_urls
urlpatterns = [
    path(
        "",
        NoteViewSet.as_view(
            {
                "get": "list",
                "post": "create",
            }
        ),
        name="note_list_create",
    ),
    path("<int:note_id>/topics/", TopicListView.as_view(), name="topic_list"),
    path("<int:note_id>/", NoteViewSet.as_view({"get": "retrieve"}), name="note_detail_update_delete"),
]

# ctrlfbe/topic_urls
urlpatterns = [
    path("", TopicCreateView.as_view(), name="topic_crete"),
    path("<int:topic_id>/pages/", PageListView.as_view(), name="page_list"),
    path("<int:topic_id>/", TopicDetailUpdateDeleteView.as_view(), name="topic_detail"),
]
  • TopicListView, TopicCreateView, TopicDetailUpdateDeleteView를 지웠기 때문에ctrlfbe/note_urls.py, ctrlfbe/topic_urls.py에서 에러가 났다.
  • 역시 TopicViewSet에 맞게 수정하자.
# ctrlfbe/note_urls.py
urlpatterns = [
    path(
        "",
        NoteViewSet.as_view(
            {
                "get": "list",
                "post": "create",
            }
        ),
        name="note_list_create",
    ),
    path(
        "<int:note_id>/topics/",
        TopicViewSet.as_view(
            {
                "get": "list",
            }
        ),
        name="topic_list",
    ),
    path("<int:note_id>/", NoteViewSet.as_view({"get": "retrieve"}), name="note_detail_update_delete"),
]

# ctrlfbe/topic_urls.py
urlpatterns = [
    path(
        "",
        TopicViewSet.as_view(
            {
                "post": "create",
            }
        ),
        name="topic_create",
    ),
    path(
        "<int:topic_id>/",
        TopicViewSet.as_view(
            {
                "get": "retrieve",
            }
        ),
        name="topic_detail",
    ),
      path("<int:topic_id>/pages/", PageListView.as_view(), name="page_list"),
]
  • Note API View 리팩토링처럼 as_view()메서드에 매핑했다.

Page API View 리팩토링은 Topic과 매우 유사하기 때문에 생략한다.

Issue도 유사하게 리팩토링 하긴 했는데, model의 성격이 Note, Topic, Page와 조금 다르다고 생각해서 나중에 따로 포스팅할 예정


4. Commits & Notion


5. 후기(?)

  • 확실히 클래스 하나로 구현해놓으니까 내가 읽기 편해서 좋다.
  • note_data, topic_data, page_data, issue_data를 만드는 코드들이 중복이 매우 많다. 리팩토링 필요
  • note, topic, page create 메서드가 data가 조금 다른것을 제외하고는 역시 중복된 코드가 많다. 리팩토링 필요
  • topic, page list 메서드 또한 note_id, topic_id가 필요하다는 것만 다르고 중복된 코드다. 리팩토링 필요
  • DRF 공식문서를 보면, ModelViewSet.as_view()를 쓰는 것보다 Router를 이용하다는것이 보편적이라고 한다.(Typical) 공부할겸 읽어보고 수정이 필요하다면 수정할 계획
  • commit을 보면 Note, Topic, Page 전부 수정하고 commit을 했는데, 모델별로 쪼개서 commit하는게 더 좋았을것 같다.
  • Notion에 쓰고 티스토리 블로그에 옮겼는데.. 불편하다. 토글기능(더보기)도 notion과 티스토리가 달라서 그대로 복붙하기 불편 ㅜ..

'Refactoring' 카테고리의 다른 글

[Refactoring] 중복 코드 제거2(상속)  (0) 2021.12.25
[Refactoring] 중복 코드 제거1  (0) 2021.12.15

+ Recent posts