Binary ↔ Decimal

 

 

자바

package temp;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

import org.junit.jupiter.api.Test;

public class ConvertDecimalAndBinaryTest {

  @Test
  public void testConvertDecimalToBinary() {
    int decimal = 21;
    String binary = Integer.toBinaryString(decimal);
    System.out.println(String.format("Decimal: %d -> Binary: %s", decimal, binary));
    assertThat(binary, is("10101"));
  }

  @Test
  public void testConvertBinaryToDecimal() {
    String binary = "1011010";
    int decimal = Integer.parseInt(binary, 2);
    System.out.println(String.format("Binary: %s -> Decimal: %s", binary, decimal));
    assertThat(decimal, is(90));
  }
}
  • Binary To Decimal: Integer.toBinaryString(int n) 메소드 사용
  • Decimal To Bianry: Integer.parseInt(String s, int radix) 메소드에 radix = 2를 넣고 사용 

 

 

 

 

 

 

파이썬은?

from unittest import TestCase


class TestConvertDecimalAndBinary(TestCase):
    def test_convert_decimal_to_binary(self):
        decimal = 74
        binary_including_0b = bin(decimal)
        binary_excluding_0b = bin(decimal)[2:]
        print(f"Decimal: {decimal} -> Binary(including 0b): {binary_including_0b}")
        print(f"Decimal: {decimal} -> Binary(excluding 0b): {binary_excluding_0b}")
        self.assertEqual(binary_including_0b, "0b1001010")
        self.assertEqual(binary_excluding_0b, "1001010")

    def test_convert_binary_to_decimal(self):
        binary = "10111110"
        decimal = int(binary, 2)
        print(f"Binary: {binary} -> Decimal: {decimal}")
        self.assertEqual(decimal, 190)
  • Binary To Decimal: int(binary, 2)
  • Decimal To Binary: bin(decimal)
    • "0b"를 빼고 싶을 때: bin(decimal)[2:] 

 

'Python' 카테고리의 다른 글

[Python] Fraction  (0) 2022.01.03
[Python] Empty String Check  (0) 2022.01.03

문제

https://www.codewars.com/kata/5a8d2bf60025e9163c0000bc

 

Codewars: Achieve mastery through coding challenge

Codewars is a coding practice site for all programmers where you can learn various programming languages. Join the community and improve your skills in many languages!

www.codewars.com

 

문제 설명

In this Kata, you will sort elements in an array by decreasing frequency of elements. If two elements have the same frequency, sort them by increasing value.

Solution.sortByFrequency(new int[]{2, 3, 5, 3, 7, 9, 5, 3, 7});

// Returns {3, 3, 3, 5, 5, 7, 7, 2, 9}
// We sort by highest frequency to lowest frequency.
// If two elements have same frequency, we sort by increasing value.

More examples in test cases.

Good luck!

 

배열에 integer 값들 count순서대로 정렬하고, count가 같다면 오름차순 정렬하라는 뜻

 

 

 

 

어떻게 풀까?

  • 정렬 문제라서 무지성으로 sort() 메소드를 써야겠다라는 생각을 함
  • 근데 counting순서로 정렬을 할 수 있나..?? sort() 메소드 쓰기전에 배열의 값들 counting을 해야할 것 같음
  • 일단 Map에 (int, count)으로 데이터를 모으고 얘네를 count순으로 정렬하고 배열에 담는 식으로 하면 될 것 같음
  • 드가자

 

 

 

 

코드

자바 코드

package codewars._6kyu.Simple_frequency_sort_20220117;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;
import java.util.stream.Collectors;

public class Solution {

  public static int[] sortByFrequency(int[] array) {
    Map<Integer, Integer> countingMap = getCountingMap(array);
    List<Entry<Integer, Integer>> entryList = getSortedEntryList(countingMap);
    List<Integer> result = getListSortedByFrequency(entryList);

    return result.stream().mapToInt(x -> x).toArray();
  }

  private static Map<Integer, Integer> getCountingMap(int[] array) {
    Map<Integer, Integer> map = new HashMap<>();
    for (int target : array) {
      map.put(target, map.getOrDefault(target, 0) + 1);
    }
    return map;
  }

  private static List<Entry<Integer, Integer>> getSortedEntryList(
      Map<Integer, Integer> countingMap) {
    List<Entry<Integer, Integer>> entryList = new ArrayList<>(countingMap.entrySet());
    entryList.sort((o1, o2) -> {
      if (o1.getValue().equals(o2.getValue())) {
        return o1.getKey() - o2.getKey();
      }
      return o2.getValue() - o1.getValue();
    });
    return entryList;
  }

  private static List<Integer> getListSortedByFrequency(List<Entry<Integer, Integer>> entryList) {
    List<Integer> result = new ArrayList<>();
    for (Entry<Integer, Integer> entry : entryList) {
      int key = entry.getKey();
      int value = entry.getValue();
      for (int i = 0; i < value; i++) {
        result.add(key);
      }
    }
    return result;
  }

  public int[] sortByFrequencyBestPracticeSolution(int[] array) {
    Map<Integer, Long> integerCountMap =
        Arrays.stream(array)
            .boxed()
            .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
    return Arrays
        .stream(array)
        .boxed()
        .sorted(Comparator.<Integer, Long>comparing(integerCountMap::get)
            .reversed()
            .thenComparing(Comparator.naturalOrder()))
        .mapToInt(Integer::intValue)
        .toArray();
  }
}

Map으로 (int, count) 데이터를 만들고, EntrySet list를 통해 value(count) 순으로 정렬 (count가 같다면 key 오름차순), 그리고 result list에 하나씩 담아 배열로 변환하여 리턴

근데 Best Practice를 보니까 Stream으로 신박하게 해결했다. 참고하면 좋을듯?

Collectors.groupingBy(), Function.identity(), Collectors.counting(), Comparator.<Integer, long>comparing() 메소드들은 처음 보는 메소드들인데 알아두면 Stream에서 요긴하게 쓸 수 있을듯

 

 

 

파이썬 코드

def solve(arr):
    int_count_map = {}
    for integer in arr:
        int_count_map.setdefault(integer, 0)
        int_count_map[integer] = int_count_map.get(integer) + 1

    sorted_by_value_dict = sorted(int_count_map.items(), key=lambda x: x[1], reverse=True)

    result = []
    for items in sorted_by_value_dict:
        key = items[0]
        value = items[1]
        for i in range(value):
            result.append(key)

    return result


def solve_best_practice(arr):
    return sorted(sorted(arr), key=lambda n: arr.count(n), reverse=True)

파이썬으로도 풀어보자 해서 억지로 풀긴했다. 근데 BestPractice보니까 헛웃음이 나더라

자바에서 푼 것 처럼 dict에 (int, count)로 담고, count 내림차순으로 정렬했다.

근데 BestPractice보니까 그냥 개 쉽게 풀어버림.. 당황스럽네

 

Fraction(a, b) → a / b가 나오는 마법

간단한 예시

fraction = Fraction(16, -10)
print(f"numerator: {fraction.numerator}")
print(f"denominator: {fraction.denominator}")

 

 

 

생성자

  • Fraction(numerator=0, denominator=1): 분자와 분모를 받아서 "분자 / 분모" 형태로 나타내는 Fraction 객체 생성, 분모가 0인경우에는 ZeroDivisionError 발생 
  • Fraction(other_fraction): 다른 Fraction 객체를 이용하여 Fraction 객체 생성
  • Fraction(float): 소수 형태(x.xxx)를 "분자 / 분모" 형태로 나타내는 Fraction 객체 생성
  • Fraction(decimal): 정수 형태(1,2 등..)를 "분자 / 1" 형태로 나타내는 Fraction 객체 생성
  • Fraction(string): 문자열 형태를 "분자 / 분모" 형태로 나타내는 Fraction 객체 생성

 

다양한 예시

Fraction(16, -10) # Fraction(-8, 5)
Fraction(123) # Fraction(123, 1)
Fraction() # Fraction(0, 1)
Fraction("3/7") # Fraction(3, 7)
Fraction("-3/7") # Fraction(-3, 7)
Fraction('1.414213 \t\n') # Fraction(1414213, 1000000)
Fraction('-.125') # Fraction(-1, 8)
Fraction('7e-6') # Fraction(7, 1000000)
Fraction(2.25) # Fraction(9, 4)
Fraction(1.1) # Fraction(2476979795053773, 2251799813685248)

 

참고

https://docs.python.org/3/library/fractions.html

'Python' 카테고리의 다른 글

[Python] Binary To Decimal, Decimal To Binary  (0) 2022.01.21
[Python] Empty String Check  (0) 2022.01.03

Empty String Check (빈 문자열 확인)

1. 자바

자바에서는 isEmpty() 메소드를 사용했다.

String name = "LeeJinHo";
if (name.isEmpty()) {
  System.out.println("이름이 없습니다!");
} else {
  System.out.println(String.format("이름은 %s입니다.", name));
}

2. 파이썬

파이썬에서는 그냥 not을 사용한다.

name = ""
if not name:
  print("이름이 없습니다")
else:
  print(f"이름은 {name}")

'Python' 카테고리의 다른 글

[Python] Binary To Decimal, Decimal To Binary  (0) 2022.01.21
[Python] Fraction  (0) 2022.01.03

이전 포스팅

 

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