Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2019.09.16 code review #1

Open
JunyongPark97 opened this issue Sep 16, 2019 · 1 comment
Open

2019.09.16 code review #1

JunyongPark97 opened this issue Sep 16, 2019 · 1 comment

Comments

@JunyongPark97
Copy link
Contributor

JunyongPark97 commented Sep 16, 2019

  1. Django 문법 관련?
  1. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L392

related_name은 category로 하는게 맞는 것 같습니다 :)

  1. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L216

실 배포시에는 print는 빼는게 좋습니다.

  1. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L392

one to one field가 나을 것 같습니다!!

  1. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/serializers.py#L182

필터 사용시 null=False가 아니라 color_source=null

  1. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/views.py#L594

filter() 자체가 쿼리셋이므로 이후 .all()할 필요 없습니다. 또한 여기 코드에서 queryset을 안쓰는 것 같은데 그 이유는 모든 코드가 CroppedImage기준으로 작성되어있어서 그렇습니다. CroppedImage기준으로 queryset을 사용하면, queryset = CroppedImage.objects.filter(categories__handle_source__isnull=True)처럼 사용할 수 있습니다. ('_' 두개로 한단계씩 들어갈 수 있음: 필터 문법)

@JunyongPark97
Copy link
Contributor Author

JunyongPark97 commented Sep 16, 2019

  1. Django 구조 관련
  1. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/views.py#L653-L656
    아까 말했듯이 client에서 받은 id로 조회해서 update하는게 더 나은 코드이다!
    또한 one to one field로 정의할 경우 .last() 할 필요 없습니다!

  2. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/views.py#L696-L702
    serializer.is_valid 같은 경우 valid가 아닌 경우 500에러가 난다. (앱인 경우 바로 튕긴다) 따라서 try except나 if 문으로 검증 후 return하는게 좋습니다.

  3. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/views.py#L639
    문법에 맞지 않음. Categories.charm_source는 쿼리셋이므로 .object를 할 수 없습니다!
    따라서 CharmTag를 가져와서 Categories모델을 업데이트 하는 코드를 짜야 합니다.

  4. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/views.py#L658-L664
    이렇게 쓰는 시도는 좋지만 여기서 사용한 serializer를 살펴보면
    https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/serializers.py#L520
    여기서 사용한 모델은 Categories이고 필드는 ForeignKey... client에서는 숫자만 주므로 .is_valid()호출 시에 에러가 날 수 밖에 없음. 따라서 serializer안에 save 메서드를 선언해서 client에서 준 id를 parse해서 Categories모델을 update하는 코드를 짜야 함. 그럼 3)도 해결!

  5. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/views.py#L641
    4)와 같이 한다면, (a.k.a. one to one field사용 한다면) charm_valid와 같은 필드를 선언할 필요 없이, Categories.charm_source 가 null인지를 확인해서 사용할 수 있습니다.

  6. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L279
    안좋은 작명센스.. (죄송합니다ㅜ) 좀 더 이쁜 작명센스 부탁드립니다.

  7. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L303
    안쓰는 코드는 빼주세요!

  8. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L245
    null=True, blank=True는 코드상 생성되는 순서를 고려해서 써주세요. 여기서는 CroppedImage는 무조건 OriginalImage가 생성된 후 하나를 물고 있기 때문에 굳이 쓸 필요가 없습니다.
    또한, 마찬가지로 하나의 OriginalImage에서 한장의 사진만 사용한다면 OnetoOneField로 Cropped를 물고있어 도 괜찮을듯? (아니면 하나의 OriginalImage에서 여러개의 CroppedImage를 생성할 수 있도록 짠다면 Best!)

  9. https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/models.py#L206-L211
    filter()를 사용했을때 해당 값이 없다면 자동으로 None 값이 나옵니다. 굳이 if 로 쓸 필요 없고, 만약 filter가 아니라 get을 사용했을때 해당 결과값이 없다면 500error가 나기 때문에 이때는 if, try except로 걸러야 합니다.
    https://github.com/mondeique/MONDE-data-server/blob/10201b303310c3eac67b1b8c773a57c8e8aa5ac8/category_data/serializers.py#L693
    마찬가지

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant