HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythondjangoMinor

Returning information about searches performed by a user

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
performeduserreturningaboutsearchesinformation

Problem

I have this simple piece of code from my django project. I feel that it isn't very well written.

Maybe it possible to write this simple function in a better way? Maybe could you give me any advices how to improve this code? I'm trying to learn how to write good quality code.

Or maybe this code is good enough?

import datetime
from accounts.models import UserProfile

def get_search_data(request):
    if request.user.is_authenticated():
        user_email = request.user.email
        user_profile = UserProfile.objects.get(user=request.user)
        user_phone = user_profile.phone_number
    else:
        user_email = ''
        user_phone = ''
    search_date = datetime.datetime.now()
    rent_starts = datetime.datetime.strptime(request.GET.get('date_start'), '%Y-%m-%d')
    rent_ends = datetime.datetime.strptime(request.GET.get('date_end'), '%Y-%m-%d')

    rent_duration = rent_ends - rent_starts
    days_till_rent_start = rent_starts - search_date

    search_date = search_date.strftime('%Y-%m-%d')
    rent_duration = rent_duration.days
    days_till_rent_start = days_till_rent_start.days

    data = {'user_email': user_email, 'user_phone': user_phone,
        'search_date': search_date, 'rent_duration': rent_duration,
        'days_till_rent_start': days_till_rent_start}

    return {'search_data': data}


I use this function to return information about searches user make on page. Function returns data in readable format for user. I give this info to javascript function. This information is used for remarketing.

Solution

The code is readable enough and to the point, there is not much to improve. I would just extract date retrieval into its own function so there is no risk of slightly mistyping the format, for instance; and change the formating of the output dict to improve readability:

import datetime
from accounts.models import UserProfile

def extract_date(request, parameter, fmt='%Y-%m-%d'):
    date = request.GET.get(parameter)
    return datetime.datetime.strptime(date, fmt)

def get_search_data(request):
    if request.user.is_authenticated():
        user_email = request.user.email
        user_profile = UserProfile.objects.get(user=request.user)
        user_phone = user_profile.phone_number
    else:
        user_email = ''
        user_phone = ''

    search_date = datetime.datetime.now()
    rent_starts = extract_date(request, 'date_start')
    rent_ends = extract_date(request, 'date_end')
    rent_duration = rent_ends - rent_starts
    days_till_rent_start = rent_starts - search_date

    return {'search_data': {
        'user_email': user_email,
        'user_phone': user_phone,
        'search_date': search_date.strftime('%Y-%m-%d'),
        'rent_duration': rent_duration.days,
        'days_till_rent_start': days_till_rent_start.days,
    }}


However, given that this function does not interact with your models (except to retrieve a phone number that could have been passed to a template earlier on), I'm wondering why you perform this computation server side. All the necessary informations are already available to the client and javascript's Date objects can perform the same kind of manipulations than what you're doing with datetime.

Code Snippets

import datetime
from accounts.models import UserProfile


def extract_date(request, parameter, fmt='%Y-%m-%d'):
    date = request.GET.get(parameter)
    return datetime.datetime.strptime(date, fmt)


def get_search_data(request):
    if request.user.is_authenticated():
        user_email = request.user.email
        user_profile = UserProfile.objects.get(user=request.user)
        user_phone = user_profile.phone_number
    else:
        user_email = ''
        user_phone = ''

    search_date = datetime.datetime.now()
    rent_starts = extract_date(request, 'date_start')
    rent_ends = extract_date(request, 'date_end')
    rent_duration = rent_ends - rent_starts
    days_till_rent_start = rent_starts - search_date

    return {'search_data': {
        'user_email': user_email,
        'user_phone': user_phone,
        'search_date': search_date.strftime('%Y-%m-%d'),
        'rent_duration': rent_duration.days,
        'days_till_rent_start': days_till_rent_start.days,
    }}

Context

StackExchange Code Review Q#156752, answer score: 2

Revisions (0)

No revisions yet.