Skip to content

Conversation

@MagerOK
Copy link
Owner

@MagerOK MagerOK commented Sep 25, 2023

No description provided.

from books.models import Book


def serialize_book(book: Book) -> dict:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут бы тайпеддикт в аннотации в идеале сделать

books/views.py Outdated
def get_all_books_json_view(request):
books = Book.objects.all()
if not books:
return JsonResponse(data={'data': {}, 'error': 'No books found'}, status=404)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для списка сущностей пустой список – штатная ситуция, не надо делать 404

year_of_publishing: int
copies_printed: int
short_description: str

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пустой строки не хватает

from books.models import Book


class SerializedBook(TypedDict):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я такие типы предпочитаю селить в отдельный файл, тогда реже циклические импорты возникают.

books/views.py Outdated
def get_all_books_json_view(request: HttpResponse) -> HttpResponse:
books = Book.objects.all()
if not books:
return JsonResponse(data={'data': {}, 'error': 'No books found'})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да говорю ж: если книг нету, это не ошибка, в дате будет пустой список, это значит шо книг нету, это норм. Избавься от этого ифа.

from books.serializer import serialize_book


class Get_All_Books_Json_View(View):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Подчёркивания лишние, классы называем КемелКейсом

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

Successfully merging this pull request may close these issues.

3 participants