[Refactoring] 중복 코드 제거1

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

Refactoring 일지 1. APIView To ModelViewSet

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

• https://github.com/ThinLineIT/CtrlF_Backend/commit/bf0dd0e69e1465b3fdd17f632c45793d7120d5af

 

4. 후기(?)

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

 

5. 참고자료

• 리팩토링
• 클린코드
• https://refactoring.guru/replace-conditional-with-polymorphism
• 클린 코더스 (17:05 부터)