How can I refactor the validation code to minimize it?

Question

I am using Django Rest Framework and below is the code of one of the API Endpoints. The code creates generates the PUBLIC and SECRET Values based on Stellar SDK.

def create(self, request):
 try:
 if 'app_id' not in request.data:
 response = {'status': 'FAIL', 'data': 'App ID field not found'}
 return Response(response, status=status.HTTP_400_BAD_REQUEST)
 if 'customer_id' not in request.data:
 response = {'status': 'FAIL', 'data': 'Customer ID field not found'}
 return Response(response, status=status.HTTP_400_BAD_REQUEST)
 customer_id = request.data['customer_id']
 app_id = request.data['app_id']
 app = App.objects.get(id=app_id)
 # Creating Stellar Account
 addr, secret = create_account()
 if addr is None and secret is None:
 response = {'status': 'FAIL', 'data': 'Could not generate customer Wallet'}
 return Response(response, status=status.HTTP_200_OK)
 # Make sure we do not add duplicate customers
 wallet_count = Wallet.objects.filter(customer_id=customer_id).count()
 if wallet_count > 0:
 response = {'status': 'FAIL', 'data': 'The customer ID already exist'}
 return Response(response, status=status.HTTP_200_OK)
 # The customer does not exist, hence create his wallet.
 wallet = Wallet(customer_id=customer_id, app=app, address=addr, secret=secret)
 wallet.save()
 if wallet.pk is None and wallet.pk > 0:
 response = {'status': 'OK', 'data': 'success'}
 else:
 response = {'status': 'FAIL', 'data': 'Messed up'}
 except App.DoesNotExist:
 response = {'status': 'FAIL', 'data': 'The App ID not found'}
 return Response(response, status=status.HTTP_404_NOT_FOUND)
 return Response({'status': 'OK', 'data': 'success'}, status=status.HTTP_200_OK)

As you can see there is repetition of codeblock like below:

response = {'status': 'FAIL', 'data': 'The customer ID already exist'}
 return Response(response, status=status.HTTP_200_OK)

It'd be nice if I can reduce all this code either by passing to a function or someway once and let it handle based on request dict.

• Heslacher: Welcome to Code Review. I have rolled back your edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .

Answer 1

Exception handling

You may want to consider reducing the try block to where the expected exception actually is raised.

try:
 app = App.objects.get(id=app_id)
except App.DoesNotExist:
 ...

Possible bugs

This test does not consider the case, that only one of addr and secret is None. Is that permitted?

if addr is None and secret is None:
 ...

These response dicts are never used:

if wallet.pk is None and wallet.pk > 0:
 response = {'status': 'OK', 'data': 'success'}
else:
 response = {'status': 'FAIL', 'data': 'Messed up'}

Deduplication

Regarding your concern of duplicated dicts: I see none. Sure, they are similar, but there are no duplicates, since the error messages differ. Of course, you can outsource the generation to a function like

def make_response(data: str, success: bool) -> dict:
 """Returns a response dict."""
 return {'status': 'OK' if success else 'FAIL', 'data': data}

but it's up to you to decide if that's worth an extra function.

• Volatil3: Thanks, Richard, your first two recommendations implemented/fixed however did not get the dedup one. Could you check the updated question?
• Stefan: You could drop the response variable, and just return - it simplifies this a bit, e.g. : return Response({'status': 'FAIL', 'data': 'Customer ID field not found'}, status=status.HTTP_400_BAD_REQUEST)
• D. Ben Knoble: I thought of writing a function to use like return fail_with('Customer ID field not found', status.HTTP_404_BAD_REQUEST)