How to refactor duplicate method inside class based views for Django 1.9

Question

I have a class based view called PalletContentProductionView

@method_decorator(PermissionManager.production_worker_required, name='dispatch')
class PalletContentProductionView(APIView):
    """
    Update a Pallet Content count and/or product
    """
    def put(self, request, pk):
        pallet_content = QuickFind.get_pallet_content_or_404(pallet_content_id=pk)

        def validate_pending_and_created_by_user(pallet_content, user=request.user):
            if not pallet_content.pending():
                raise PermissionDenied("Non-Pending pallet content cannot be updated by production worker")
            EntryFormHelper.throw_403_when_not_created_by_user(pallet_content=pallet_content, user=user)
        return update_pallet_content(request, pallet_content, validate_pending_and_created_by_user)

    """
    Delete pallet only if pallet content is pending and creater by user
    """
    def delete(self, request, pk):
        pallet_content = QuickFind.get_pallet_content_or_404(pallet_content_id=pk)

        def validate_pending_and_created_by_user(pallet_content, user=request.user):
            if pallet_content.pending() is False:
                raise PermissionDenied("Non-Pending pallet content cannot be updated by production worker")
            EntryFormHelper.throw_403_when_not_created_by_user(pallet_content=pallet_content, user=user)
        return delete_pallet_content(request, pallet_content, validate_pending_and_created_by_user)

I was wondering how do I refactor such that the four lines of duplication can be avoided?

The four lines are :

def validate_pending_and_created_by_user(pallet_content, user=request.user):
        if not pallet_content.pending():
            raise PermissionDenied("Non-Pending pallet content cannot be updated by production worker")
        EntryFormHelper.throw_403_when_not_created_by_user(pallet_content=pallet_content, user=user)

I have tried to pull it out and make it inside the PalletContentProductionView and adjacent to the put and delete methods but it was wrong.

You've already got it as a function, so why not just extract it to class level? — Sayse, Dec 19 '16 at 08:22

score 1 · Accepted Answer · answered Dec 19 '16 at 08:29

Just create a method that generates the corresponding function:

@method_decorator(PermissionManager.production_worker_required, name='dispatch')
class PalletContentProductionView(APIView):
    def put(self, request, pk):
        pallet_content = self.get_pallet_content(pk)
        return update_pallet_content(
            request, pallet_content,
            self.generate_callback(request, pk)
        )

    def delete(self, request, pk):
        pallet_content = self.get_pallet_content(pk)
        return delete_pallet_content(
            request, pallet_content,
            self.generate_callback(request, pk)
        )

    def get_pallet_content(self, pallet_content_pk):
        return QuickFind.get_pallet_content_or_404(pallet_content_id=pk)

    def generate_callback(self, request, pallet_content):
        def validate_pending_and_created_by_user(pallet_content, user=request.user):
            if not pallet_content.pending():
                raise PermissionDenied("Non-Pending pallet content cannot be updated by production worker")
            EntryFormHelper.throw_403_when_not_created_by_user(pallet_content=pallet_content, user=user)
        return validate_pending_and_created_by_user

Note that here we also use a get_pallet_content to avoid the duplication of QuickFind.get_pallet_content_or_404(pallet_content_id=pk).

Also, note that you should use if not something: instead of if something is False:.

it works exactly as you described. thank you. I am not very familiar with this way of callback. Any online resources or books you can recommend so I can better understand why this works? — Kim Stacks, Dec 19 '16 at 09:18
There absolutely no magic implied. the `generate_callback` method just returns a function. This function is in turn passed as an argument to the `delete_pallet_content` and `update_pallet_content` functions, just like you did in your code. If you are new to Python I suggest you take a look at existing learning resources: https://www.python.org/about/gettingstarted/ — Régis B., Dec 19 '16 at 10:32
Hi Regis, sorry for circling back. Why do you recommend `if not something` instead of `if something is False` — Kim Stacks, Dec 22 '16 at 05:42
I didn't mean to make a blanket statement, `is False` should sometimes be used but only in edge cases. I.e: when you want to check that an object is really equal to `False`, and not just "falsy". More details here: http://stackoverflow.com/questions/9494404/use-of-true-false-and-none-as-return-values-in-python-functions#9494887 — Régis B., Dec 22 '16 at 07:53

How to refactor duplicate method inside class based views for Django 1.9

1 Answers1