Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is currently the code that I'm using to validate and check for errors in a flask app, where the form fields are coming from a /registration html page:

@app.route("/register", methods=["GET", "POST"])
def register():
    if request.method == "POST":
        if request.form['register'] == 'Register':
            if request.form.get("eulacheck") != "on":
                return regError("Please read and agree to the Terms and Conditions and the License Terms.")
            if request.form['username'] == "":
                return regError("Please select a username.")
            if request.form['password1'] == "":
                return regError("Please enter a password.")
            if request.form['password2'] == "":
                return regError("Please confirm password.")
            if request.form['password1'] != request.form['password2']:
                return regError("Passwords did not match. Please enter passwords again.")
            username = request.form['username']
            password = request.form['password1']
            hash = bcrypt.hashpw(password.encode('UTF-8'), bcrypt.gensalt())
            id = randint(1000,9999)
            if request.form['email1'] != "":
                if request.form['email2'] == "":
                    return regError("Please confirm email.")
                if request.form['email1'] != request.form['email2']:
                    return regError("Email addresses did not match. Please enter emails again.")
                email = request.form['email1']
                insert_db("INSERT INTO Users (ID, Username, Email, Hash) VALUES (?, ?, ?, ?)", (id, username, email, hash))
                flash("Account registration successfull! Please use your credentials to login below.")
                return render_template("login.html",pageType=['login'],flashType="info")
            insert_db("INSERT INTO Users (ID, Username, Hash) VALUES (?, ?, ?)", (id, username, hash))
            flash("Account registration successfull! Please use your credentials to login below.")
            return render_template("login.html",pageType=['login'],flashType="info")
    return render_template("register.html",pageType=['register'])


def regError(message):
    flash(message)
    return render_template("register.html",pageType=['register'],flashType="danger")

This code seems a bit messy and repetitive, is there a better way that I should be doing this? Thanks a lot!

share|improve this question

1 Answer 1

Looking at register(), the format is as following:

def register():
    if request.method == "POST":
        if request.form['register'] == 'Register':
            stuff()
    return render_template("register.html",pageType=['register'])

This can easily be condensed using and:

def register():
    if request.method == "POST" and request.form['register'] == 'Register':
            stuff()
    return render_template("register.html", pageType=['register'])

Also, you are not inserting whitespaces between arguments.

stuff(3,3)  # <--Unconventional
stuff(3, 3) # <--More conventional

Another stylistic issue is some of your lines are too long. For all lines, line.length <= 79. You can read about the specific python conventions on the Python website.
For making it look less messy and repetitive, look at this answer to a Stack Overflow question. It is about implementing switch and case statements into Python, which seems perfect for your situation.

share|improve this answer
1  
Yup, I can definitely make the first and change, and I'll watch out for the whitespaces between arguments. Thanks a lot for the help! –  cloudcrypt Dec 25 '14 at 20:41

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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