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

Django controller method to list the top characters, guilds, and killers

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

Problem

I've been reading the Chapter 8 of this book.

Here's the view I'm currently coding. It list all characters/guilds according to the type of URL.

/ranking -> List Top 50 Characters

/ranking/guild -> List top guilds

/ranking/pvp -> list top killers


def ranking(request, type):
    if type=='':
        queryset_list = Character.objects.annotate(num_resets=Count('resets')).order_by('-resets').select_related('account')[:50]
        paginator = Paginator(queryset_list, 10)

        page = request.GET.get('page')
        try:
            queryset = paginator.page(page)
        except PageNotAnInteger:
            queryset = paginator.page(1)
        except EmptyPage:
            queryset = paginator.page(paginator.num_pages)
            context = {'object_list' : queryset, 'states':STATES, 'class_type':CLASS_TYPE}
        template = 'ranking_character.html'
    elif type=='guild':
        queryset_list = Guild.objects.all()
        paginator = Paginator(queryset_list, 10)

        page = request.GET.get('page')
        try:
            queryset = paginator.page(page)
        except PageNotAnInteger:
            queryset = paginator.page(1)
        except EmptyPage:
            queryset = paginator.page(paginator.num_pages)

        context = {'object_list' : queryset}
        template = 'ranking_guild.html'

    elif type=='pvp':
        queryset_list = Character.objects.filter(pk_count__gt=0).annotate(total=Count('pk_count')).order_by('-pk_count')[:50]
        paginator = Paginator(queryset_list, 10)

        page = request.GET.get('page')
        try:
            queryset = paginator.page(page)
        except PageNotAnInteger:
            queryset = paginator.page(1)
        except EmptyPage:
            queryset = paginator.page(paginator.num_pages)
            context = {'object_list' : queryset, 'class_type':CLASS_TYPE}
        template = 'ranking_pvp.html'

    return render(request,template,context)


It's possible to make it more "generic"? How ca

Solution

I think you can just make a function to get the queryset and re-use it:

def queryset_from_page():
    page = request.GET.get('page')
    try:
        return paginator.page(page)
    except PageNotAnInteger:
        return paginator.page(1)
    except EmptyPage:
        return paginator.page(paginator.num_pages)


Also why is context being defined at different indentation levels?

except EmptyPage:
        queryset = paginator.page(paginator.num_pages)

    context = {'object_list' : queryset}


vs

except EmptyPage:
        queryset = paginator.page(paginator.num_pages)

        context = {'object_list' : queryset, 'class_type':CLASS_TYPE}


In one case you define context everytime, other times only when a particular branch in the exception handling is taken: is this a bug?

Code Snippets

def queryset_from_page():
    page = request.GET.get('page')
    try:
        return paginator.page(page)
    except PageNotAnInteger:
        return paginator.page(1)
    except EmptyPage:
        return paginator.page(paginator.num_pages)
except EmptyPage:
        queryset = paginator.page(paginator.num_pages)

    context = {'object_list' : queryset}
except EmptyPage:
        queryset = paginator.page(paginator.num_pages)

        context = {'object_list' : queryset, 'class_type':CLASS_TYPE}

Context

StackExchange Code Review Q#129851, answer score: 2

Revisions (0)

No revisions yet.