Fetching and displaying the provisioning status of a phone number

Posted on

Problem

I have the following function:

def dnd_return(number,my_timeout):
    try:
        r =requests.get("https://dndcheck.p.mashape.com/index.php?mobilenos= {}".format(number), timeout =my_timeout)
    except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as err:
        return "Error fetching DND info"
    else:
        return r.text

Then I have the following route:

@app.route('/track_mobile_numbers', methods=['POST','GET'])
def mobile():
    if request.method == 'POST':
        if request.form.get("number", "") != "":
            number = request.form['number']
            number = number.replace(" ","")
            number = number.replace('+91', '')
            number = number.strip()
            strnumber = number
            dnd_list = []
            if len(number) == 10:
                try:
                    dnd_dump = dnd_return(number,9)
                    json_dump = json.loads(dnd_dump)
                    j = json_dump[0]

                    if j['DND_status'] != "deactivated" and j['DND_status'] != "off":
                        dnd_list.append("DND status : "+j['DND_status'])
                        dnd_list.append("Date of Activation : "+j['activation_date'])
                        dnd_list.append("Preference : "+j['current_preference'])

                    else:
                        dnd_list.append("DND status : "+j['DND_status'])
                except Exception as e:
                    dnd_list.append(e)
            else:
                dnd_list.append("The number should contain 10 digits")

            number = number[:4]
            try:
                number = int(number)
                num = mobileNumbers.query.filter_by(number = number).first()
                return render_template('mobilenumbers.html', 
                                        strnumber = "Mobile number : " + strnumber, 
                                        operator = "Operator : " + num.operator, 
                                        state = "State : " + num.state,
                                        dnd = dnd_list
                                      )
            except Exception as err:
                return render_template('mobilenumbers.html',
                                        error = err
                                      )
        else:
            return render_template('mobilenumbers.html',
                                    error = "Please enter a valid number"
                                  )
    else:
        return render_template('mobilenumbers.html')

Is there a better way to do this? Can you recommend improvements to this code?

Solution

  • You may want to use a different template for error messages. You can still reuse the overall design by inheriting from a base template.
  • You replace variable values a lot, making it hard to follow what the meaning of variables like num, number and strnumber are throughout the code. Instead write functions which create the various transformations of the input that you need, making your variables unchanged throughout the code. For example:

    def format_number(number):
        return number.replace(" ","").replace('+91', '').strip()
    

    Now you can get from number to strnumber without ever changing the former.

  • In some error cases you render the template with error set, but in at least one you use a different error message delivery: dnd_list.append("The number should contain 10 digits"). This seems inconsistent.
  • The various large branches of code should be moved into separate functions.
  • Single letter variables like e, r and j make this code hard to maintain. Yes, it’s kinda obvious from context, but the less context you have to keep in your head while developing, the better.

Leave a Reply

Your email address will not be published. Required fields are marked *