开发者

how should i refactor a piece of code that is present in many different functions, but has return statement in it

The code snippet is coming from django view code, but it doesn't matter much.

Let's say I have the following piece of code...

def unsubscribe(request):
  #start of block
  user = request.user
  sid = request.POST.get('subscription_id')
  try:
    sub = Subscription.objects.get(id=sid)
  except ObjectDoesNotExist:
    return ajax_response(False, [('subsc开发者_StackOverflow中文版ription', 'Given subscription does not exist.')])
  if sub.user != user:
    return ajax_response(False, [('subscription', 'Invalid permission.')])
  #end of block.
  sub.is_active = False
  sub.save()
  return ajax_response(True)

and say, I have another function resubscribe() that does exactly same thing as the above function, except it does sub.is_active = True.

In that case, what would be the best way to organize the code, so that code between #block and #endblock aren't duplicated? I guess in general, this question can be written as:

There's a block of code that is copy-and-pasted over many different functions. however, this block of code contains return statement. in that case, what would be the best way to abstract this piece of logic out?

EDIT: fixed code snippet.

EDIT2: Actually, the easy way to solve this question is by creating a function say, toggle_active_status, which takes request and a boolean. (I figured it out after posting out the question).

However, i'm wondering about the case where the different logic between the functions is more than 1 line... like, say, code between #block and #endlbock only does input validation, and potentially any arbitrary application logic can come after.


Factor out the duplicated chunk and have the difference be passed in, creating wrapper functions that pass in the difference:

def alter_subscription(request, make_active):
  # start of block
  # (...)
  # end of block
  sub.is_active = make_active
  sub.save()
  return ajax_response(True)

def unsubscribe(request):
  return alter_subscription(request, False)

def resubscribe(request):
  return alter_subscription(request, True)


A couple options that haven't been shown yet:

Since this is a verification routine, use exceptions to indicate failure, and a normal None (ignored) return to indicate success:

def verify_permissions(request):
  user = request.user
  sid = request.POST.get('subscription_id')
  try:
    sub = Subscription.objects.get(id=sid)
  except ObjectDoesNotExist:
    raise PermissionError, 'Given subscription does not exist.'
  if sub.user != user:
    raise PermissionError, 'Invalid permission.'

def subscribe(request):
  try:
    verify_permissions(request)
    sub.save()
    return ajax_response(True)
  except PermissionError, why:
    return ajax_response(False, [('subscription', why)])

Or, since Python is dynamically typed and it's apparently OK to call ajax_response in a couple different ways: return the arguments used to construct the response, and check the value of the first one.

def verify_permissions(request, purpose):
  user = request.user
  sid = request.POST.get('subscription_id')
  try:
    sub = Subscription.objects.get(id=sid)
  except ObjectDoesNotExist:
    return (False, [(purpose, 'Given subscription does not exist.')]) 
  if sub.user != user:
    return (False, [(purpose, 'Invalid permission.')])
  return (True,)

def subscribe(request):
  result = verify_permissions(request, 'subscription')
  if result[0]: sub.save()
  return ajax_response(*result)


One way is to simply create a separate function containing the duplicated functionality. You can use the parameters of the function to introduce the necessary differences.

If the code snippet returns something, then you might use a convention, such as to return None if the function should not return, or an object otherwise. In your caller function, just test whether the returned parameter is None or not, and return the returned variable if it isn't None.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜