CODEWITHSUNDEEP
pythondjangopython-3.xdjango-rest-framework
Abu Sayem
Abu Sayem

Reputation: 518

How do i write this code more pythonic way

I am writing Django's views to check if a valid album id is provided or not. But it looks cumbersome and hard to read. I want to make it more readable and short. I cant directly check data['id']. It will give error if its blank.

def post(self, request):
    if len(request.body) > 0:
        data = json.loads(request.body.decode('utf-8'))
    else:
        return Response({'message': 'Album id is required'})

    if 'id' not in data:
        return Response({'message': 'Valid Album id is required'})

    try:
        id = int(data['id'])
        if id < 1:
            return Response({'message': 'Valid album id is required'})
        album = Album.objects.get(pk=id)
    except:
        return Response({'message': 'Valid album id is required'})

Upvotes: 0

Views: 61

Answers (2)

juanpa.arrivillaga
juanpa.arrivillaga

Reputation: 95948

The biggest two things to keep in mind when trying to increase readability is don't prematurely optimize and use functions to encapsulate logic (especially if that logic may have to be re-used in the future).

So, just a quick example:

def post(self, request):
    if request.body:
        data = json.loads(request.body.decode('utf-8'))
    else:
        return Response({'message': 'Album id is required'})

    if _validate_data(data):
        id_ = int(data['id'])
        album = Album.objects.get(pk=id_)
        # probably want to return a Response as well...
    else:
        return Response({'message': 'Valid Album id is required'})


def _validate_data(data):
    if 'id' not in data:
        return False
    try:
        id_ = int(data['id'])
    except ValueError:
        return False
    return id_ >= 1

There's different approaches you could take. For example, instead of _validate_data you could have _extract_id which returns None when the data is invalid, otherwise an id_, which you would then check, so something like:

id_ = _extract_id(data)
if id_ is None:
    return Response({'message': 'Valid Album id is required'})
else:
    # do the rest of the stuff

You must decide what makes more sense given the rest of your code and any standards you are following.

As a quick aside, this doesn't seem like a post to me, because it seems like you are just retrieving data, the album by id, POST is generally used to create new resource.

Upvotes: 1

afterburner
afterburner

Reputation: 2781

The whole problem seems to be what to do when the id is not a valid entry in data.

Looking at your code, and particularly the part below, it seems that an id value less than 1 is invalid.

 if id < 1:
    return Response({'message': 'Valid album id is required'})

Furthermore, the try...except means that, if id is not part of data, then that is also invalid.

This means your code could be simplified by using default values, as such:

def post(self, request):
    if len(request.body) > 0:
        data = json.loads(request.body.decode('utf-8'))
    else:
        return Response({'message': 'Album id is required'})

    id = int(data.get('id', 0))
    if id < 1:
        return Response({'message': 'Valid album id is required'})
    album = Album.objects.get(pk=id)
    # f-strings below are a python 3
    return Response({'message': f'The album you have requested is {album}'})

The important part of the snippet above is: data.get('id', 0) which returns data['id'] if id is a valid field, and 0 if id is not found inside data

Unrelated, because both are valid ways of checking request.body, but the more pythonic way is to do it like @juanpa.arrivillaga suggested, namely: if request.body:

To further simplify it, I would reduce some of the duplication by doing it this way:

def post(self, request):
    if request.body:
        data = json.loads(request.body.decode('utf-8'))

        id = int(data.get('id', 0))
        if id < 1:
            return Response({'message': 'Valid album id is required'})
        album = Album.objects.get(pk=id)
        # f-strings below are a python 3
        return Response({'message': f'The album you have requested is {album}'})

    return Response({'message': 'Album id is required'})

Upvotes: 1

Related Questions