Как избавиться от дублирования кода?

Question

[YuriyCherniy]

Возникла необходимость расширить контекстные данные в паре классов. Переопределил метод get_context_data, в результате чего получил два одинаковых метода в разных классах. Посоветуйте какой подход использовать в данном случае для соблюдения принципа DRY?

class ItemListView(ListView):
    model = Item
    paginate_by = 9

    def get_context_data(self, **kwargs):
        '''
        Add phone number to context data
        '''
        user = User.objects.get(pk=1)
        data = super().get_context_data(**kwargs)
        try:
            data['phone_number'] = user.userprofile.phone_number
        finally:
            return data


class ItemDetailView(DetailView):
    model = Item

    def get_context_data(self, **kwargs):
        '''
        Add phone number to context data
        '''
        user = User.objects.get(pk=1)
        data = super().get_context_data(**kwargs)
        try:
            data['phone_number'] = user.userprofile.phone_number
        finally:
            return data

Comments

[Модератор]

YuriyCherniy дорогой пользователь, настоятельно рекомендуем еще раз обратить самое пристальное внимание на п. 3.1 регламента работы сервиса (и, в особенности, на его последний абзац).
В противном случае ваши вопросы будут удаляться по причине тег-спама, а систематические нарушения приведут к блокировке учетной записи.

Answer 1

[antonksa]

from django.views.generic import View, ListView, DetailView


class AddUserPhoneToContextMixin(View):
    def get_context_data(self, **kwargs):
        '''
        Add phone number to context data
        '''
        ctx = super().get_context_data(**kwargs)
        try:
            user = User.objects.get(pk=1)
            ctx['phone_number'] = user.userprofile.phone_number
        finally:
            return ctx


class ItemListView(AddUserPhoneToContextMixin, ListView):
    model = Item
    paginate_by = 9


class ItemDetailView(AddUserPhoneToContextMixin, DetailView):
    model = Item

Только не очень понятно почему только pk=1, что будет если его нет, и почему в try-except вы обернули попытку доступа к userprofile. Еще считается, что except без указания конкретных классов обрабатываемых исключений это bad practice. В таком случае вы теряете контроль над пониманием, какое конкретно исключение произошло - то ли коннект в базе оборвался, то ли юзер не существует, то ли поля такого нет у его профиля.

Хорошая практика - это указывать конкретно что бы обрабатываете:

try:
    phone_number = user.userprofile.phonenumber
except UserProfile.DoesNotExists:
    raise NotFound('Warning! User profile did not created!')

Comments

[YuriyCherniy]

Только не очень понятно почему только pk=1

Регистрация на сайте не предусмотрена, только администратор, которого создает тот, кто деплоит сайт. Поэтому pk=1 захардкожен.

почему в try-except вы обернули попытку доступа к userprofile

userprofile существует по умолчанию, его тоже создаёт тот кто деплоит сайт. Проверяется попытка доступа к user.userprofile.phone_number, которого может не быть, но контекст вернуть необходимо в любом случае.

Еще считается, что except без указания конкретных классов обрабатываемых исключений это bad practice.

flake8 не ругается на конструкцию try/finally подумал, что линтер больше меня знает как надо.

Буду рад комментариям если я всё таки наговнокодил.

[antonksa]

но контекст вернуть необходимо в любом случае.

абсолютно верное рассуждение. всегда так и делайте. всегда думайте где ваш код может повести себя экстренным образом и предусматривайте обработку этих веток логики.

Проверяется попытка доступа к user.userprofile.phone_number, которого может не быть

А вот такого не может быть. phone_number это поле модели. Если связанный userprofile существует, то у него всегда будут ВСЕ поля. Т.е. если значение не будет указано, то в поле будет храниться пустая строка, и в контекст запишется пустая строка. В вашем случае эта логика некорректна, а следовательно избыточна. А вот обернуть в try-except получение админа очень желательно, при этом ограничить обработку только одним классом - DoesNotExist. Ну и предусмотреть какое-то сообщение типа "Админ не создан, прочитайте пункт 1.5 инструкции."

В целом вы не наговнокидили. Тут полно дебилов, которые задают тупейшие вопросы, а потом еще и бросаются с кулаками, что за них не все написали, а только ссылку на решение предоставили или намекнули куда бежать. Дерзайте, учитесь, побольше кодите, опыт можно приобрести только самомтоятельно.

[YuriyCherniy]

А вот такого не может быть. phone_number это поле модели.

Действительно, делаю одно, а в голове держу другое.

Спасибо за уделённое время, очень полезно по всем пунктам.