AS-IS Code

def get_board_set_from_board_group(request, board_group_id):
    ...

def home(request):
    ...

def board(request, board_url):
    ...

def post_detail(request, board_url, pk):
    ...

def reply_write(request, board_url, pk):
    ...

def rereply_write(request, board_url, pk):
    ...

def reply_delete(request, board_url, pk):
    ...

def rereply_delete(request, board_url, pk):
    ...

def like(request, board_url, pk):
    ...

Board 앱의 View 함수들을 살펴보니 총 9개의 코드가 존재합니다.
하나하나씩 불필요한 코드를 제거하거나 리팩토링해 보겠습니다.

 

---

 

get_board_set_from_board_group

def get_board_set_from_board_group(request, board_group_id):
    try:
        board_group = BoardGroup.objects.get(
            id=board_group_id
        )
    except BoardGroup.DoesNotExist:
        raise Http404

    board_set = board_group.board_set.all().values('name', 'url')

    return HttpResponse(json.dumps({'board_set': list(board_set)}), 'application/json')

우선 리팩토링을 하기 전에 이 코드가 무슨 역할을 하고 있는지 확인이 필요합니다.

Json 으로 Response 를 전달하고 있는 것 같습니다.

아래 이미지 처럼 게시판 그룹들이 있는데, 이걸 클릭할 때 나타나는 하위 정보를 json 으로 전달하는 것 같습니다.

 

자 이제 무엇을 하는지 알게 되었으니 리팩토링을 해볼까요?

• 불필요한 코드를 확인합니다.
• 중복되는 코드가 있는지 확인합니다.
• View를 정리하여 필요하다면 Service Layer에 함수를 모읍니다.
• 불필요하게 여러 번 호출되는 쿼리가 있다면 리팩토링합니다.
• 테스트 케이스를 작성합니다.

 

불필요한 코드를 확인합니다.

    ...
    try:
        board_group = BoardGroup.objects.get(
            id=board_group_id
        )
    except BoardGroup.DoesNotExist:
        raise Http404

    board_set = board_group.board_set.all().values('name', 'url')
    ...

제 눈에는 이 코드가 불필요해 보입니다. 아래와 같이 바꿀 수 있지 않을까요?

def get_board_set_from_board_group(request, board_group_id):
    board_info = Board.objects.filter(
        board_group_id=board_group_id
    ).values(
        'name',
        'url',
    )
    return HttpResponse(json.dumps({'board_set': list(board_info)}), 'application/json')

JSON으로 응답을 줄 것인데 굳이 404를 반환할 필요도 없고, 만약 없는 값이면 빈 리스트로 내려주면 그만인데, 불필요한 try-except를 이용하고 있었습니다.

 

중복되는 코드가 있는지 확인합니다. (없어 Pass)

 

View를 정리하여 필요하다면 Service Layer에 함수를 모읍니다.

바뀐 코드를 봅시다.

board_info = Board.objects.filter(
        board_group_id=board_group_id
    ).values(
        'name',
        'url',
    )

이 코드는 여기뿐만 아니라 다른 View에서도 사용할 수 있습니다. 그러면 Service Layer를 Board App에서 하나 만들어서 이것을 따로 함수로 분리하면 추후에 다른 곳에서 이용하기 편하고 중복 코드도 없어질 것입니다.

services.py 파일 안에 아래 있는 코드를 작성합니다.

from typing import List

from board.models import Board


def get_boards_by_board_group_id(board_group_id: int) -> List[Board]:
    return list(
        Board.objects.filter(
            board_group_id=board_group_id,
        )
    )

 

불필요하게 여러 번 호출되는 쿼리가 있다면 리팩토링합니다. (없어 Pass)

 

테스트 케이스를 작성합니다.

이제 없던 테이스 케이스를 작성해봅시다.

새로 만든 services 내부에 있는 코드의 테스트 케이스를 작성합시다.

 

저는 테스트케이스 작성할 때 Given, When, Then 방식으로 테스트를 작성합니다.

https://www.agilealliance.org/glossary/given-when-then/

from django.test import TestCase

from board.models import BoardGroup, Board
from board.services import get_boards_by_board_group_id


class GetBoardsByBoardGroupIdTestCase(TestCase):
    def setUp(self):
        self.group1 = BoardGroup.objects.create(group_name='group1')
        self.group2 = BoardGroup.objects.create(group_name='group2')
        self.board1_with_group1 = Board.objects.create(
            url='board1',
            name='board1',
            board_group=self.group1,
        )
        self.board2_with_group1 = Board.objects.create(
            url='board2',
            name='board2',
            board_group=self.group1,
        )
        self.board3_with_group2 = Board.objects.create(
            url='board3',
            name='board3',
            board_group=self.group2,
        )
        self.board4_without_group = Board.objects.create(
            url='board4',
            name='board4',
        )

    def test_get_boards_by_board_group_id_when_board_has_group(self):
        # Given: Board group ids
        given_board_group_ids = [self.group1.id, self.group2.id]
        expected_boards = [
            [self.board1_with_group1, self.board2_with_group1],
            [self.board3_with_group2],
        ]

        for given_board_group_id, expected_board in zip(given_board_group_ids, expected_boards):
            # When: Get boards by board group id
            boards = get_boards_by_board_group_id(given_board_group_id)

            # Then: Boards are returned
            self.assertEqual(boards, expected_board)

    def test_get_boards_by_board_group_id_when_board_group_not_exists(self):
        # Given: Delete all board groups
        Board.objects.all().update(board_group=None)

        # When: Get boards by board group id
        boards = get_boards_by_board_group_id(self.group1.id)

        # Then: No boards are returned
        self.assertEqual(boards, [])

 

View 코드를 추가로 바꿉시다.

def get_board_set_from_board_group(request, board_group_id):
    boards = get_boards_by_board_group_id(board_group_id)
    context = [{'name': b.name, 'url': b.url} for b in boards]
    return HttpResponse(json.dumps({'board_set': context}), 'application/json')

물론 이렇게 끝낼 수 있습니다! 하지만!!!! 응답용 DTO를 만들 겁니다.

지금은 코드가 짧아서 확인하기 쉽습니다.
하지만 코드가 길어지면 context 부분과 반환하는 부분이 멀어지면, '이게 어떻게 응답되는 거지?'라는 의문이 듭니다.
이를 알기 위해서는 context가 만들어지는 부분에서 무엇이 일어나는지 알아야 합니다.
그렇지 않고 만약 저희가 'context는 이런 형태를 가지고 있어!'라고만 알려주면 어떨까요?|
코드로 한번 보시죠.

파일은 dtos 라는 폴더 하위에 response_dtos.py 를 생성합니다.

dto 는 pydantic 을 이용해서 작성하려고 합니다.

from pydantic import (
    BaseModel,
    Field,
)


class BoardSetGroupResponse(BaseModel):
    board_set: list = Field(...)


class BoardSetBoardInfo(BaseModel):
    name: str = Field(...)
    url: str = Field(...)

위와 같이 Resposne DTO 를 만듭니다. 

 

그러면 아래와 같이 코드를 수정할 수 있습니다.

def get_board_set_from_board_group(request, board_group_id: int):
    return HttpResponse(
        BoardSetGroupResponse(
            board_set=[
                BoardSetBoardInfo(
                    name=_board.name,
                    url=_board.url
                ) for _board in get_boards_by_board_group_id(board_group_id)
            ]
        ).model_dump_json(),
        'application/json',
    )

 

AS-IS 와 TO-BE 를 비교해 봅시다.

AS-IS

def get_board_set_from_board_group(request, board_group_id):
    try:
        board_group = BoardGroup.objects.get(
            id=board_group_id
        )
    except BoardGroup.DoesNotExist:
        raise Http404

    board_set = board_group.board_set.all().values('name', 'url')

    return HttpResponse(json.dumps({'board_set': list(board_set)}), 'application/json')

TO-BE

바꾸는 김에 View 의 명칭도 수정했습니다.

def get_boards_info_from_board_group(request, board_group_id: int):
    return HttpResponse(
        BoardSetGroupResponse(
            board_set=[
                BoardSetBoardInfo(
                    name=_board.name,
                    url=_board.url
                ) for _board in get_boards_by_board_group_id(board_group_id)
            ]
        ).model_dump_json(),
        'application/json',
    )

두 코드를 비교해보면, AS-IS 코드를 볼 때는 '왜 이렇게 했지?'라는 의문이 듭니다. 반면, TO-BE 코드를 보면 '왜 이렇게 했지?'보다는 '응답은 어떻게 나올까?'라는 생각이 먼저 듭니다.

물론 사람마다 자신의 스타일이 다르기 때문에 무언가를 고집하지는 않습니다.
그럼에도 불구하고 읽기 쉬운 코드를 작성하는 것이 좋은 코드 개발이라고 생각합니다.
위에 있는 것은 제가 생각하는 방식이기에 다른 사람에게 강요하지 않습니다.
하지만 이런 방법도 있다고 생각합니다.

테스트 케이스를 작성합니다.

지금은 Service Layer 까지만 테스트코드를 작성했습니다.
클라이언트가 Endpoint 접근할 때의 테스트 케이스도 작성해야합니다.

이 첫 View 만 테스트 케이스를 작성하려고 합니다.
나머지 함수에 대해서는 View 테스트 케이스는 조금 특별한 게 아닌 이상 넘어가도록 하겠습니다.

test_views.py 파일을 생성 후 아래 코드를 작성했습니다.

import json

from django.test import TestCase
from django.urls import reverse

from board.models import Board, BoardGroup
from board.views import get_boards_info_from_board_group


class BoardGroupTestCase(TestCase):
    def setUp(self):
        # 테스트에 필요한 데이터 세팅
        self.group1 = BoardGroup.objects.create(group_name='group1')
        self.group2 = BoardGroup.objects.create(group_name='group2')
        self.board1_with_group1 = Board.objects.create(
            url='board1',
            name='board1',
            board_group=self.group1,
        )
        self.board2_with_group1 = Board.objects.create(
            url='board2',
            name='board2',
            board_group=self.group1,
        )
        self.board3_with_group2 = Board.objects.create(
            url='board3',
            name='board3',
            board_group=self.group2,
        )
        self.board4_without_group = Board.objects.create(
            url='board4',
            name='board4',
        )
        self.url = reverse(
            'board:get_boards_info_from_board_group',
            args=[self.group1.id],
        )

    @staticmethod
    def _create_url(group_id: int):
        return reverse(
            'board:get_boards_info_from_board_group',
            args=[group_id],
        )

    def test_get_boards_info_from_board_group_with_existing_group(self):
        # Given: 그룹1에 속한 게시판들을 조회하는 요청 데이터
        url = self._create_url(self.group1.id)

        # When: HTTP GET 요청
        response = self.client.get(url)
        content = json.loads(response.content.decode())

        # Then: HTTP 응답
        self.assertEqual(response.status_code, 200)
        # And: 응답 데이터 검증
        self.assertEqual(
            content,
            {
                'board_set': [
                    {
                        'name': self.board1_with_group1.name,
                        'url': self.board1_with_group1.url,
                    },
                    {
                        'name': self.board2_with_group1.name,
                        'url': self.board2_with_group1.url,
                    },
                ]
            },
        )

    def test_get_boards_info_from_board_group_without_existing_group(self):
        # Given: 그룹을 전부 없앰
        Board.objects.all().update(board_group=None)
        # And: 그룹1에 속한 게시판들을 조회하는 요청 데이터
        url = self._create_url(self.group1.id)

        # When: HTTP GET 요청
        response = self.client.get(url)
        content = json.loads(response.content.decode())

        # Then: HTTP 응답
        self.assertEqual(response.status_code, 200)
        # And: 응답 데이터 검증
        self.assertEqual(
            content,
            {
                'board_set': []
            },
        )

 

지금까지 생성한 테스트케이스에 이슈가 없습니다!

이제 모든 리펙토링이 get_board_set_from_board_group 한정해서 끝났습니다!!!

 

0 0