'TDD' 태그의 글 목록

TDD

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

[Refactoring] 중복 코드 제거2(상속)

2021. 12. 25. 15:19

이전 포스팅

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 상수..? 이정도의 간단한 수준이기 때문에 다형성을 이용한 리팩토링은 좀 오래 걸릴것 같다고 겁(?)을 조금 먹었다.
그럼에도 불구하고 나에게는 테스트코드가 있다. 이것 저것 시도해보고 잘 안되면 원복하고 "아, 이 방법은 좀 더 공부가 필요하거나, 여기서는 적용하기 힘들겠다;"하고 블로그나 노트에 정리만 잘 해놓으면 된다. (비록 그 삽질동안 잡아먹는 시간은 아까울수 있지만..)
바로 리팩토링을 시작하기 전에 대충 구조를 상상했는데 다음과 같다.

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

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

리팩토링 시작

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}

각 ModelViewSet의 create() 메소드에서 request를 인자로 받도록 했다.
이에 따라 ModelViewSet의 create() 메소드도 조금 수정했다.

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.py의 build_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

[Refactoring] 중복 코드 제거1

2021. 12. 15. 18:22

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

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

4. 후기(?)

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

5. 참고자료

저작자표시 (새창열림)

'Refactoring' 카테고리의 다른 글

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

PREV 1 NEXT

내 블로그 - 관리자 홈 전환	`Q` `Q`
새 글 쓰기	`W` `W`

글 수정 (권한 있는 경우)	`E` `E`
댓글 영역으로 이동	`C` `C`

이 페이지의 URL 복사	`S` `S`
맨 위로 이동	`T` `T`
티스토리 홈 이동	`H` `H`
단축키 안내	`Shift` + `/` `⇧` + `/`

Dev 일기장

TDD

[Refactoring] 중복 코드 제거2(상속)

이전 포스팅

이번에 할 리팩토링

리팩토링 시작

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

2. `basedata.py` 수정

3. `create()` 메소드 수정

4. `if else` statement 삭제

커밋

후기

'Refactoring' 카테고리의 다른 글

[Refactoring] 중복 코드 제거1

1. build_data() 메소드 추출

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

3. Commit

4. 후기(?)

5. 참고자료

'Refactoring' 카테고리의 다른 글

+ Recent posts

티스토리툴바

개인정보

단축키

내 블로그

블로그 게시글

모든 영역

Dev 일기장

TDD

[Refactoring] 중복 코드 제거2(상속)

이전 포스팅

이번에 할 리팩토링

리팩토링 시작

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

2. basedata.py 수정

3. create() 메소드 수정

4. if else statement 삭제

커밋

후기

'Refactoring' 카테고리의 다른 글

[Refactoring] 중복 코드 제거1

1. build_data() 메소드 추출

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

3. Commit

4. 후기(?)

5. 참고자료

'Refactoring' 카테고리의 다른 글

+ Recent posts

티스토리툴바

개인정보

단축키

내 블로그

블로그 게시글

모든 영역

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

2. `basedata.py` 수정

3. `create()` 메소드 수정

4. `if else` statement 삭제