정시훈 과제 03 제출 #92
정시훈 과제 03 제출 #92
Conversation
| if Tag.objects.filter(name=tag): | ||
| _tag = Tag.objects.get(name=tag) | ||
| comment.tags.add(_tag) | ||
| else: | ||
| _tag = Tag(name=tag) | ||
| _tag.save() | ||
| comment.tags.add(_tag) |
There was a problem hiding this comment.
쿼리가 중복으로 날아가지 않도록 짜는 것이 좋습니다.
| if Tag.objects.filter(name=tag): | |
| _tag = Tag.objects.get(name=tag) | |
| comment.tags.add(_tag) | |
| else: | |
| _tag = Tag(name=tag) | |
| _tag.save() | |
| comment.tags.add(_tag) | |
| tagObj = Tag.objects.filter(name=tag).first() | |
| if tagObj is None: | |
| tagObj = Tag(name=tag) | |
| tagObj.save() | |
| comment.tags.add(tagObj) |
| if Tag.objects.filter(name=tag): | ||
| _tag = Tag.objects.get(name=tag) | ||
| instance.tags.add(_tag) | ||
| else: | ||
| _tag = Tag(name=tag) | ||
| _tag.save() | ||
| instance.tags.add(_tag) |
There was a problem hiding this comment.
마찬가지로 쿼리가 중복으로 날아가는 코드는 피해주세요.
| def update(self, instance, validated_data): | ||
|
|
||
| instance.content = validated_data.get('content', instance.content) | ||
| instance.updated_at = validated_data.get('updated_at', instance.updated_at) |
There was a problem hiding this comment.
auto_now=True 이면 자동으로 해주는 걸로 아는데 이 코드는 빼도 될 것 같습니다.
| from django.utils import timezone | ||
|
|
||
| from .models import User, Post, Comment, Tag | ||
| from . import views |
There was a problem hiding this comment.
views와 serializers가 서로 종속성을 갖고 있네요.
import 문도 하나의 statement로 해석되는 파이썬에서 순환종속성은 가급적 피하는 게 좋습니다.
당장은 문제가 없지만, 나중에 순환종속성으로 인한 문제가 발생하면 골치가 아픕니다.
| fields = ['username', 'password'] | ||
|
|
||
| class TagSerializer(serializers.ModelSerializer): | ||
| name = serializers.CharField(max_length=200) |
There was a problem hiding this comment.
�ModelSerializer 를 썼으니 이건 없어도 될 듯 합니다.
| def TagCheck(): | ||
| tags = Tag.objects.all() | ||
| for tag in tags: | ||
| if tag.post_set.all(): | ||
| pass | ||
| elif tag.comment_set.all(): | ||
| pass | ||
| else: | ||
| tag.delete() |
There was a problem hiding this comment.
이건,, 매우 비효율적인 코드네요. 게시글 또는 댓글을 작성할 때마다 이렇게 무거운 쿼리를 3번씩이나 날린다는 건 좋지 않습니다.
차라리 comment나 post를 삭제할 때, 그들이 참조하고 있던 태그가 있다면 해당 태그에 대해서만 검사하는 게 그나마 나을 것 같습니다.
다른 방법이라면 tag 내에 ref_count 같은 필드를 두고 이 값이 0이 될때만 삭제하는 방법도 있겠네요.
| path('api/posts', APIPostList.as_view(), name='api-post-list'), | ||
| path('api/posts/<int:pk>', APIPostDetail.as_view(), name='api-post-detail'), | ||
| path('api/posts/<int:pk>/comments', APICommentList.as_view(), name='api-comment-list'), | ||
| path('api/posts/<int:pk>/comments/<int:comment_pk>', APICommentDetail.as_view(), name='api-comment-detail'), |
There was a problem hiding this comment.
이렇게 해도 상관은 없지만, 여기서 post의 id는 불필요한 값이므로 아래처럼 단축하는 것도 방법입니다.
api/comments/<int:comment_pk>
| from django.test import TestCase | ||
|
|
||
| # Create your tests here. |
There was a problem hiding this comment.
이번 과제 스펙에는 없었지만, 앞으로는 테스트를 꼭 작성하시는 게 좋습니다.
심지어, TDD라는 개발 방법론에서는 테스트를 먼저 작성하고 그 다음 개발을 시작할 것을 권하고 있을 정도로 테스트의 중요성은 매우 높습니다. 관심이 있으시면 한 번 찾아보시기 바라요
|
|
||
| def perform_create(self, serializer): | ||
| serializer.save(created_by=self.request.user) | ||
| return super().perform_create(serializer) |
There was a problem hiding this comment.
이 return 문에 있는 코드는 제가 보기엔 불필요해보이는데 사용하신 이유가 있다면 말씀해주세요.
| 'rest_framework.authentication.BasicAuthentication', | ||
| 'rest_framework.authentication.SessionAuthentication', | ||
| 'rest_framework.authentication.TokenAuthentication', # <-- And here |
There was a problem hiding this comment.
이렇게 하면 위에서부터 순차적으로 BasicAuthentication, SessionAuthentication, TokenAuthentication 가 순차적으로 실행되며 인증 여부를 판단합니다. 이렇게 여러 개의 인증을 거치는 것은 지금은 오버스펙이기 때문에 TokenAuthentication 만 빼고 나머지는 지워주는 것이 좋겠습니다.
Waffle03-env.eba-r4zqv8aj.ap-northeast-2.elasticbeanstalk.com
pr리뷰 부탁드립니다.
추가로 api 링크에서 하나 적지 않은 게 있어 여기다 적습니다.
api/<post_id>/comments/<comments_id>를 통해 댓글에 접근할 수 있습니다.