What’s wrong with [your_favourite_web

This example is taken directly from Flask tutorial but you will likely find similar examples for other frameworks as well. What does it do? Well, as the name implies it registers a user: validates the data, saves changes in the database and presents the result back to the user:

@bp.route('/register', methods=('GET', 'POST'))
def register():
    if request.method == 'POST':
        username = request.form['username']
        password = request.form['password']
        db = get_db()
        error = None

        if not username:
            error = 'Username is required.'
        elif not password:
            error = 'Password is required.'
        elif db.execute(
            'SELECT id FROM user WHERE username = ?', (username,)
        ).fetchone() is not None:
            error = f"User {username} is already registered."

        if error is None:
            db.execute(
                'INSERT INTO user (username, password) VALUES (?, ?)',
                (username, generate_password_hash(password))
            )
            db.commit()
            return redirect(url_for('auth.login'))

        flash(error)

    return render_template('auth/register.html')

I'm aware that tutorials are meant to be as simple to understand as possible, but I feel there is a danger that you will copy this pattern in your next project.

So what's good about this code? It works and it's simple to understand.

So what's wrong with it? It lacks Separation of Concerns. And it doesn't follow Single-responsibility Principle. These principles are simple:

don’t write your program as one solid block, instead, break up the code into chunks that are finalized tiny pieces of the system each able to complete a simple distinct job,
every class (or function) in your code should have only one job to do.

It's very likely that in the near future your controller will have to include even more logic (i.e. sending email notification to the user, creating another user-related object upon registration, etc.).

Maybe in a couple of weeks, the business will be back to you saying: "Now we also need to create users via our brand new API. And the command line.". Now you will need to somehow run the same code from multiple endpoints, or as a reaction to a specific system event.

You may have heard the phrase "keep your controllers thin". The key idea is to keep controllers as simple as possible and move the business logic to another layer of abstraction - namely, the application layer. Let's refactor the original route handler from the tutorial:

@bp.route('/register', methods=('GET', 'POST'))
def register():
    if request.method == 'POST':
        username = request.form['username']
        password = request.form['password']
        is_success, error = create_user(username, password)
        if is_success:
            return redirect(url_for('auth.login'))
        else:
            flash(error)

    return render_template('auth/register.html')

As you can see create_user is now responsible for executing a command that creates a new user account. All the implementation details are hidden inside this function and now it will be much easier to execute this command from other places (i.e. REST endpoint, batch processing, etc.).

Also, our controller is now as thin as possible. In a web app, the controllers should be concerned with HTTP verbs and they should focus on processing the request input, executing a specific command (in case of operations that change the state of the system), and presenting results back to the user. In DDD terms, the controllers are part of the infrastructure layer. On the other hand create_user function is a functional requirement of the system, therefore it is located in the application layer.

Let's get back to create_user function. We could implement it as follows:

def create_user(username, password):
    db = get_db()

    error = None

    if not username:
        return False, 'Username is required.'
    elif not password:
        return False, 'Password is required.'
    elif db.execute(
        'SELECT id FROM user WHERE username = ?', (username,)
    ).fetchone() is not None:
        return False, f"User {username} is already registered."

    db.execute(
        'INSERT INTO user (username, password) VALUES (?, ?)',
        (username, generate_password_hash(password))
    )
    db.commit()
    return True, None

There is still one problem with our code. create_user is now responsible both for validating user data and database operation. We could further separate those concerns by introducing validate_user_command function:

def validate_user_command(username, password):
    if not username:
        return False, 'Username is required.'
    elif not password:
        return False, 'Password is required.'
    return True, None


def create_user(username, password):
    db = get_db()
    error = None

    ok, error = validate_user_command()
    if error:
        return False, error

    if db.execute(
        'SELECT id FROM user WHERE username = ?', (username,)
    ).fetchone() is not None:
        return False, f"User {username} is already registered."

    db.execute(
        'INSERT INTO user (username, password) VALUES (?, ?)',
        (username, generate_password_hash(password))
    )
    db.commit()
    return True, None

As you see we are validating data in the create_user function. Another possibility would be to call validation logic in the controller, just before calling create_user but I prefer to do it as a part of a command execution flow. It is far more common to have the same validation for different application entry points: web and API for example. Having a validation logic directly in the controller means that the logic is controller-specific, rather than application-specific.

You might also wonder why am I returning (is_success, error) result tuple as opposed to raising an exception. I'd say it's a matter of preference - they both have their pros and cons, but this subject deserves its own article.

What’s wrong with [your_favourite_web_framework] tutorials?