6
\$\begingroup\$

I'm working on a Django Twilio app, and I believe it's coming along well. I just want to improve my code writing skills. How can I improve the below code?

def save_number(request):
    from_number = request.POST.get('From', '')
    try:
        obj = SaveNumber.objects.get(phone_number=from_number)
        if obj == obj:
            subscribe = request.POST.get('Body', )
            if subscribe.lower().strip() == "stellar":
                enter = SaveNumber.objects.get(phone_number=from_number)
                enter.subscribed = True
                enter.save()
                r = twiml.Response()
                r.message("You're in. Always text outtahere, and we'll remove you.")
                return r
            elif subscribe.lower().strip() == "outtahere":
                remove = SaveNumber.objects.get(phone_number=from_number)
                remove.subscribed = False
                remove.save()
    except SaveNumber.DoesNotExist:
        obj = SaveNumber(phone_number=from_number)
        obj.save()
        r1 = twiml.Response()
        r1.message("Reply with stellar and you're in! :-)")
        return r1
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Here is my alternative. Your original code calls the DB for the same record twice (potentially) -- the get_or_create does the same as your try/except and creates the record.

def save_number(request):
    from_number = request.POST.get('From', '')
    subscribe = request.POST.get('Body', '').lower().strip()    
    obj, created = SaveNumber.objects.get_or_create(phone_number=from_number)
    response = twiml.Response()

    if created:
        response.message("Reply with stellar and you're in! :-)")
    else:
        if subscribe == "stellar":
            obj.subscribed = True
            obj.save()
            response.message("You're in. Always text outtahere, and we'll remove you.")
        elif subscribe == "outtahere":
            obj.subscribed = False
            obj.save()
            response.message("You're out")
    return response

Disclaimer: i didnt actually test it theyre may be bugs

\$\endgroup\$
5
  • \$\begingroup\$ you can move number.save() to the outter else as well. \$\endgroup\$
    – Ali
    Jan 21, 2016 at 18:17
  • \$\begingroup\$ @Ali i would say no, there are only two places where you update the record, they should have saves() otherwise when subcribe is NOT stellar or outtahere youre making a UPDATE to the DB with the same data.. i dont know. i stand by my decision \$\endgroup\$ Jan 21, 2016 at 18:19
  • \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and how it improves upon the original) so that the author can learn from your thought process. \$\endgroup\$ Jan 21, 2016 at 18:20
  • \$\begingroup\$ @Mat'sMug ahh. My bad, i'll update it now \$\endgroup\$ Jan 21, 2016 at 18:21
  • \$\begingroup\$ Te code worked beautifully! It's my understanding that the try/except will help with error reporting. \$\endgroup\$ Jan 21, 2016 at 19:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.