[Django] westagram 코드 리뷰 1차

코드 리뷰를 받고서 고쳐야될 점들을 정리해보자.
우선 구현은 model->view->url 이였다면 코드 리뷰는 다시 원래 사용자가 접속하는 순서대로 url->view->model 순서로 진행했다.

urls.py

먼저 프로젝트의 urls.py의 코드를 점검했다.

# westagram_project/urls.py
from django.urls import path, include

urlpatterns = [
    path('account', include('account.urls')),
    path('comment', include('comment.urls')),
]

별로 고칠게 없었다. 혹시라도 url부분에 '/' 가 붙었는지 확인하는 작업이었다.

그 다음은 각각의 앱의 urls.py를 확인해봤다.

# account/urls.py

from django.urls import path
from .views import SignInView, SignUpView

urlpatterns = [
    path('/sign-up', SignUpView.as_view()),
    path('/sign-in', SignInView.as_view()),
]

이 부분은 localhost:8000/account/sign-up 이런식으로 들어오게 설정하는 부분이다.
그런데 /account/sign-up 이 부분을 나누지않고 그냥 앱의 메인페이지에 포함하는 방법도 있다. 이건 개발자 재량이라고 할 수 있다.
path('', SignUpView.as_view())
이렇게 바꾸면 /account 로 접속만해도 회원가입 뷰를 호출할 수 있다.
그 다음 comment의 urls.py의 경로들을 확인해보자.

# comment/urls.py 수정전

from django.urls import path
from .views import CommentView

urlpatterns = [
    path('/comments', CommentView.as_view()),
]

일단 경로가 중복되었다. 수정전처럼 하면 localhost:8000/comment/comments 라는 식의 주소로 들어가야 코멘트 관련 뷰를 호출할 수 있을것이다. url이 이쁘지않게 중복되었다.
그냥 path('', CommentView.as_view()) 이렇게 바꿔서 comment 에 들어오기만 해도 코멘트들을 볼 수 있게 하면 된다.

# comment/urls.py 수정후

from django.urls import path
from .views import CommentView

urlpatterns = [
	path('', CommentView.as_view()),
]

---

views.py

이제 경로에 대한 부분은 다 수정을 했다. 이제 view를 확인해보자.
먼저 account 앱의 view에서 코드리뷰 받은 부분이다.

# account/views.py 의 SignUpView 부분 수정 전

import json

from django.views import View
from django.http import HttpResponse, JsonResponse

from .models import Account


class SignUpView(View):
    def post(self, request):
        signup_data = json.loads(request.body)
        try:
            if Account.objects.filter(email=signup_data['email']).exists():
                return HttpResponse(status=409)

            Account.objects.create(
                email=signup_data['email'],
                password=signup_data['password'],
            )
            return HttpResponse(status=200)

        except KeyError:
            return JsonResponse({"message": "INVALID_KEYS"}, status=400)

먼저 클래스명 SignUpView 부분이다. UserView 를 더 추천하며, 한정적인 클래스명을 짓지말라는 말을 들었다. SignUpView 같이 딱 하나의 기능만 작동할듯한 클래스명보다, UserView 로 하면 해당 클래스내에서 좀 더 다양한 코드를 작성할 수 있을 것이다.

두번째는 변수명인 signup_data 이다. 해당 데이터가 무엇이 들어가는지 명시해주는 시도는 좋았지만, 실제로 코드로 작성하다보면 이름이 길다보니 상당히 귀찮아진다고 하셨다.

그다음에 try..excepy 문에서 filter().exists()를 get() 보다 더 선호하는 이유에 대해서 명확히 알고가라고 하셨다. 이 코드를 다른 사람이 쓰니까 쓰는게 아니라(찔린다 ㅠㅠ), 명확한 이유를 알고 상황에 맞게 써야된다고하셨다.

그래서 filter().exists() vs get() 에 대해서 이 글에 정리해봤다.

# account/views.py 의 SignUpView 부분 수정 후
class SignUpView(View):
    def post(self, request):
        data = json.loads(request.body)
        try:
            if Account.objects.filter(email=data['email']).exists():
                return HttpResponse(status=409)

            Account.objects.create(
                email=data['email'],
                password=data['password'],
            )
            return HttpResponse(status=200)

        except KeyError:
            return JsonResponse({"message": "INVALID_KEYS"}, status=400)

그 다음으로 SignInView 부분을 보자.

# account/views.py 의 SignInView 부분 수정 전
class SignInView(View):
    def post(self, request):
        signin_data = json.loads(request.body)

        try:
            if Account.objects.filter(email=signin_data['email']).exists():
                user = Account.objects.get(email=signin_data['email'])

                if user.password == signin_data['password']:
                    return HttpResponse(status=200)

                return HttpResponse(status=401)

            return HttpResponse(status=400)

        except KeyError:
            return JsonResponse({"message": "INVALID_KEYS"}, status=400)

먼저 signin_data 라는 변수명은 위와 동일하다. 길면 귀찮아진다.
그리고 중간 if문을 보면 user에 담는 식으로 코드를 작성했는데, 그렇게해도괜찮고 아예 밑에 if문에 한 번에 표현하는식으로도 가능하다고 하셨다.
또한 return 중간중간에 한 줄 띄우는것은 그냥 붙이라고 하셨다.
if문의 마지막 return은 계정이 틀렸을 때의 리턴인데, 400에러가 아니라 401도 괜찮다는 피드백을 받았다.

# account/views.py 의 SignInView 부분 수정 후
class SignInView(View):
    def post(self, request):
        signin_data = json.loads(request.body)

        try:
            if Account.objects.filter(email=signin_data['email']).exists():
                user = Account.objects.get(email=signin_data['email'])

                if user.password == signin_data['password']:
                    return HttpResponse(status=200)
                return HttpResponse(status=401)
            return HttpResponse(status=401)

        except KeyError:
            return JsonResponse({"message": "INVALID_KEYS"}, status=400)

여기까지 account 앱의 리뷰는 끝났다.
comment앱의 view는 고칠점보다는 개념을 확인하고서 코드를 썼는지 확인받았다. 아직 코드를 외우듯 치는 느낌이 있어서 파고들면서 질문을 하시면 제대로 대답을 못했다. 열심히 공부하자..

---

models.py

models.py에서는 내가 email 정보를 받는 경우 email은 고유하기 때문에, unique=True 옵션을 주라고 말씀하셨다.

email = models.CharField(max_length=200, unique=True)

그외에는 하려다가 말았던 부분을 실수로 코드에 남겨둬서 그 코드 한 줄 지우는 정도였다.

---

기타 정리할 사항들

나뿐만 아니라 다른분들이 장고 코드리뷰를 하면서 들었던 말들을 간단하게 정리해봤다.

• 경로를 쓸 때는 동사가 아니라 명사를 쓰고, 해당 클래스 네이밍도 어느정도 비슷하게 가자.
• user는 그냥 내부 경로를 아예 안주고 하기도 한다. 줄거면 /sign-up 같이 줘라. (url 경로는 단어 연결시 - 를 쓴다.)
• 엔드포인트 경로의 주소 앞에 '/'를 주는 것이다.
• 주석은 완성된 코드단에서 필요없다. 지워버리자.
• 클래스 이름은 UserView 같이 성격을 보여줘야되고, 할 일(동사)를 보여주면 안된다.
• 되도록 else는 생략하고 코드를 작성하자. 그냥 인덴트를 쓰자.
• 변수에 저장하지 않아도 되는 경우는 저장하지 말자. 괜히 메모리를 더 쓰게 된다.
• 성공시에는 HttpResponse(status=200) 정도로 충분하다.
• 프론트엔드와 서버 통신하기위해 특정 문자보다 코드화된 메시지로 반환하는게 좋다.
  • ({"message":"USER_ALREADY_EXIST"}), status=401)
• 미리 프론트단과 이야기가 되있지 않는 이상, HttpResponse(status=401) 같이 에러메시지만 보내자. 너무 자세히 알려주면 해킹당할 위험이있다.
• 회원가입시 특수한 검증(글자수, 특수문자 등)을 했다면 로그인때는 굳이 추가로 할 필요없다. 이미 가입할 때 걸어졌기 때문이다.
• JsonResponse를 통해서 메세지와 같이 상태도 보내주는것은 보안적으로도, 데이터 측면으로도 낭비되는 부분이 있다. 혹시 프론트와 이야기가 되있다면 상관없지만..
  • 물론 INVALID_KEYS 에러 일때는 JsonResponse를 하는게 좋다.
• 유저의 상세정보같은 클래스명은 UserDetails 같은식으로 주면된다.
• setting.py에 APPEND_SLASH = False 를 하면 런서버를 돌릴 때, 슬래시를 빼라는 경고가 안뜬다.

---

코드리뷰 받은 부분에 대해서 정리가 끝났다!
이제 다음 단계인 인증&인가에 대해서 공부 후에 덧붙이는 시간이다.