[SOLVED] Is there a more pythonic/efficient way to write this code

Issue

if request.method == "POST":
    user = None
    users = User.query.all()
    for x in users:
        if x.email == request.form["email"] and check_password_hash(x.password, request.form["password"]):
            user = x
    if not user:
        return render_template("login.html",err = "Invalid Credentials")
    else:
        login_user(user)
        return redirect(url_for("home"))
else:
    if current_user.is_authenticated:
        return redirect(url_for("home"))

I’m always finding myself setting a variable = None then later checking if the variable is still None like the example above. I feel like there are way better ways to write this but I can’t think of any. Any help is appreciated

Solution

Instead of querying the database for all users and then looping through all the results in your application, let the database do the work.

Here’s how I’d rewrite your code snippet (I’m guessing you’re using Flask-SQLAlchemy):

if request.method == "POST":
    user = User.query.filter_by(email=request.form["email"]).first()
    if not user or not check_password_hash(user.password, request.form["password"]):
        return render_template("login.html", err="Invalid Credentials")
    login_user(user)
    return redirect(url_for("home"))

if current_user.is_authenticated:
    return redirect(url_for("home"))

Some things to note:

• simplify your flow control and avoid unnecessary nesting
• in your code, you’re looping through all users in the database, even after you’ve found the user in question. Make sure to use break and continue statements to avoid unnecessary work
• avoid manually implementing logic for tasks that databases are built for (e.g. querying and filtering data)

Answered By – chuckx

Answer Checked By – David Marino (BugsFixing Volunteer)