Subclassing TemplateView with Mixins - a bad idea?

51 Views Asked by At

I have several "Listing" views which are very similar and I feel like I'm unecessarily repeating myself. Subclassing seems like the answer, but I've run into problems down the line before when subclassing things in Django so I'd like to ask before doing so.

If I have 2 views, like this, which use the same template, a variable message and different querysets:

class MyGiverView(LoginRequiredMixin, TemplateView):

    template_name = "generic_listings.html"
    message = ""

    def get_context_data(self, **kwargs):

        context = super().get_context_data(**kwargs)
        context["list_of_stuff"] = MyModel.objects.filter(
            giver=self.request.user,
            status=1,
        )
        context["message"] = self.message

        return context

class MyTakerView(LoginRequiredMixin, TemplateView):

    template_name = "generic_listings.html" # this hasn't changed
    message = ""

    def get_context_data(self, **kwargs):

        context = super().get_context_data(**kwargs)
        context["list_of_stuff"] = MyModel.objects.filter(
            taker=self.request.user, # this has changed
            status__in=(1,2,3), # this has changed
        )
        context["message"] = self.message # this hasn't changed

        return context

Am I going to screw it up by creating a base class like:

class MyBaseView(LoginRequiredMixin, TemplateView):

    template_name = "generic_listings.html"
    
    def get_context_data(self, **kwargs):

        context = super().get_context_data(**kwargs)
        context["list_of_stuff"] = self.qs
        context["message"] = self.message

        return context

And using it in my views as such:

class MyGiverView(MyBaseView):

  qs = MyModel.objects.filter(
    giver=self.request.user,
    status=1,
  )
  message = ""

class MyTakerView(MyBaseView):

  qs = MyModel.objects.filter(
    taker=self.request.user,
    status__in=(1,2,3),
  )
  message = ""

It's DRYer, but I'm unsure of the implications regarding what goes on "under the hood".

1

There are 1 best solutions below

0
vinkomlacic On

Instead of creating a base class, it is better to create a mixin class for what you need.

Also, using ListView would be better for what you need.

This is my suggestion:

class InjectMessageMixin:
    message = ""

    # Always use this method to get the message. This allows user
    # to override the message in case it is needed to do on a request
    # basis (more dynamic) instead of using the class variable "message".
    def get_message(self):
        return self.message

    def get_context_data(self, **kwargs):
        # This line is super important so you can normally use
        # this mixin with other CBV classes.
        context = super().get_context_data(**kwargs)
        context.update({'message': self.get_message()})

        return context


class MyGiverView(LoginRequiredMixin, InjectMessageMixin, ListView):
    # Even though this doesn't change I would keep it repeated. Normally,
    # templates are not reused between views so I wouldn't say it's necessary
    # to extract this piece of code.
    template_name = "generic_listings.html"
    message = "giver message"

    def get_queryset(self):
        return MyModel.objects.filter(
            giver=self.request.user,
            status=1,
        )


class MyTakerView(LoginRequiredMixin, InjectMessageMixin, ListView):
    # Even though this doesn't change I would keep it repeated. Normally,
    # templates are not reused between views so I wouldn't say it's necessary
    # to extract this piece of code.
    template_name = "generic_listings.html"
    message = "taker message"

    def get_queryset(self, **kwargs):
        return MyModel.objects.filter(
            taker=self.request.user, # this has changed
            status__in=(1,2,3), # this has changed
        )