Normally the way to refactor is to proceed step by step. For example, you might start with the extract method combined with the decomposition of conditionals.
This would result in:
def _ranking_without_type(request):
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'
return context, template
def _ranking_by_guild(request):
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'
return context, template
def _ranking_by_pvp(request):
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 context, template
def ranking(request, type):
if type=='':
context, template = _ranking_without_type(request)
elif type=='guild':
context, template = _ranking_by_guild(request)
elif type=='pvp':
context, template = _ranking_by_pvp(request)
return render(request,template,context)
Run the tests, and check that they still pass. (you have tests, don't you?)
In the next step I would do what Caridorc proposed.
You would have the following code:
def _queryset(request, paginator):
page = request.GET.get('page')
try:
queryset = paginator.page(page)
except PageNotAnInteger:
queryset = paginator.page(1)
except EmptyPage:
queryset = paginator.page(paginator.num_pages)
return queryset
def _ranking_without_type(request):
queryset_list = Character.objects.annotate(num_resets=Count('resets')).order_by('-resets').select_related('account')[:50]
paginator = Paginator(queryset_list, 10)
queryset = _queryset(request, paginator)
context = {'object_list' : queryset, 'states':STATES, 'class_type':CLASS_TYPE}
template = 'ranking_character.html'
return context, template
def _ranking_by_guild(request):
queryset_list = Guild.objects.all()
paginator = Paginator(queryset_list, 10)
queryset = _queryset(request, paginator)
context = {'object_list' : queryset}
template = 'ranking_guild.html'
return context, template
def _ranking_by_pvp(request):
queryset_list = Character.objects.filter(pk_count__gt=0).annotate(total=Count('pk_count')).order_by('-pk_count')[:50]
paginator = Paginator(queryset_list, 10)
queryset = _queryset(request, paginator)
context = {'object_list' : queryset, 'class_type':CLASS_TYPE}
template = 'ranking_pvp.html'
return context, template
def ranking(request, type):
if type=='':
context, template = _ranking_without_type(request)
elif type=='guild':
context, template = _ranking_by_guild(request)
elif type=='pvp':
context, template = _ranking_by_pvp(request)
return render(request,template,context)
How about refactoring that "template"?
def ranking(request, type):
template = 'ranking_{0}.html'.format(type if type != '' else 'character')
if type=='':
context = _ranking_without_type(request)
elif type=='guild':
context = _ranking_by_guild(request)
elif type=='pvp':
context = _ranking_by_pvp(request)
return render(request,template,context)
now the ranking* functions don't return the template anymore, you can remove the second return type from the ranking_* functions.
Finally, you could use a dispatch mapping, e.g., :
ranking_by_types = {
'': _ranking_without_type,
'guild': _ranking_by_guild,
'pvp', _ranking_by_pvp,
}
def ranking(request, type):
template = 'ranking_{0}.html'.format(type if type != '' else 'character')
context = ranking_by_types[type](request)
return render(request,template,context)
As you can see, functions will be pretty short. IMHO, they should in general not be longer than 10 lines, although some authors claim they should be at most 3 lines long (others up to 20).