DEV Community

Cover image for MVC Tutorials Are Broken
Ovid
Ovid

Posted on

MVC Tutorials Are Broken

Note: The Model, View, Controller pattern, or “MVC”, actually comes in many flavors, but for the purposes of this article, I’m referring to MVC for the Web. Feel free to comment below, but keep in mind that we have a particular viewpoint here.

The Problem: getting MVC wrong is a common—and expensive—error.

When I start working with a new client, I often find software with common sets of mistakes. One egregious mistake is the misunderstanding of the Model-View-Controller (“MVC”) pattern. MVC—which has gotten an unfair bad rap—is thoroughly misunderstood and much of this can be blamed on rubbish examples and tutorials.

Here’s a perfect example of bad MVC using Python and Flask:

@app.route('/')
def main_page():
    """Searches the database for entries, then displays them."""
    db = get_db()
    cur = db.execute('select * from entries order by id desc')
    entries = cur.fetchall()
    return render_template('index.html', entries=entries)
Enter fullscreen mode Exit fullscreen mode

In this example, we have a page which finds “entries” and displays them on a Web page. It’s entirely possible that you see no problem with this, but it perfectly demonstrates what’s wrong with people’s understanding of MVC: you’ve pushed knowledge of your database, your business logic, and your HTML into the controller.

MVC examples and tutorials are teaching developers how to build unmaintainable software. As a result, when I work with a new client using “MVC”, I often see this:

def some_controller_method():
    """begin hundreds of lines of code"""
Enter fullscreen mode Exit fullscreen mode

How do I hate thee? Let me count the ways. (with apologies to Elizabeth Barrett Browning). The following comes from years of experience seeing this over and over again.

  • The database or ORM is exposed at the controller level
  • You can’t reuse that logic outside of the controller
  • You can’t test that logic outside of a Web context
  • Tests are much slower because of the Web context
  • Test coverage suffers because it’s hard to test conditional logic in the controller
  • The logic often gets duplicated in other controllers
  • Different behaviors become tightly coupled, making it hard to develop
  • Performance suffers because you can’t cleanly target a layer for optimization
  • Integrating new technologies is harder because the layers are tightly coupled
  • ... and so on

If the above list seems huge, that's because this is a terrible problem that costs companies a lot of money. It's one of the worst problems in modern web application development. But what does this mean in practice? Let’s consider a real example I encountered a few years ago.

One company I worked with had all of the above problems. In particular, they had a 3,000 line “method” for their search engine. It conflated many things, including searching for products and filtering the search results. Thus, even though I was tasked with working on it, every time I touched search, it would break filtering. Every time I touched filtering, it would break search. It was slow and tedious to get anything done. Further, that method generated HTML that was injected into a Web page!

It took me some time to separate search, filtering, and moving HTML generation into the view. When I was done, I had a surprise visit from one of the developers of the mobile side. It seems they wanted a proper search engine for ages but never added it because they couldn’t because their needs were so different from the Web context. After this work, it only took them fifteen minutes to implement a search engine.

Let me repeat that because it’s important: the mobile app had never had a proper search engine because the Web app had pushed all of the logic into a Web controller. Fixing that allowed the mobile team to get a search engine running in fifteen minutes.

Not getting this right flushes money down the toilet. Often we hear that a new feature will take a lot of time to implement “because the application wasn’t designed for this use case.” What was actually meant is “because we didn’t understand separation of concerns and this simple feature will take months to develop.”

Note: you can fix this mess, but it takes time and you have to understand what MVC should look like.

A Proper Controller Method

To contrast this, let’s look at a the controller for an extremely complex bit of logic from the free MMORPG, Tau Station, using Perl and the Catalyst MVC framework:

sub station ( $self, $c, $station ) : Local Args(1) {
    $c->character->travel_to_station($station);
}
Enter fullscreen mode Exit fullscreen mode

Without going into detail about what travel_to_station does (trust me, it’s complex), all we see is a request for a complex action to be performed. In the controller we can see:

  • Knowledge of the route to this controller method (the Local attribute)
  • But no knowledge of how the model is constructed
  • And no knowledge of how the data will be used

Having the route information hard-coded is a bit unfortunate, but that’s easy to work around. However, this controller method merely connects the model to the view, and that’s all. That’s what you’re looking for in MVC. MVC, when appropriate, takes an application with a UI (user interface), and decouples the responsibilities into reasonable layers.

It’s now easy to test the logic (you’re testing the model, not the controller layer and all of its Web context). It’s easy to reuse this code. And, if the model is properly built, you have a great separation of concerns that makes it easy to maintain and extend the application.

  • Model—The application.
  • View—What the consumer (user) sees. Often HTML, JSON, or XML.
  • Controller—A thin layer connecting the View and the Model.

And that’s it! There’s nothing fancy, but it often requires explanation. The controller, as shown above, should be as thin as possible. The view should only have display logic and the model is ... what, exactly?

The Model

So let’s dig into the model. Almost every MVC tutorial that I see confuses the data model with the Model. Think of the model as “the business logic”, not the data. This is what we get (assume all following examples are pseudo-code):

@app.route('/neighbors')
def neighbors():
    location = self.character.location
    neighbors = Character.query.filter_by(location_id=location.id).all()
    return render_template('people.html', people=neighbors)
Enter fullscreen mode Exit fullscreen mode

Now that looks reasonable. In fact, it seems so sensible that I used to write my MVC code like that because that's what tutorials taught me. In the case of Tau Station, the above was deeply flawed. The above query tightly couples a controller method to the underlying structure of the database. If you need to change that structure, oops! But it’s just one method call, so factoring it out is silly, right?

Wrong. As it turns out, you didn’t want to show any characters who hadn’t been active in the past day.

Unless they were NPCs.

Or the other characters are not visible.

Any time we discover a new case of when characters are or are not visible to a given viewer, we can revisit this logic in every controller method which needs to know this or we can abstract it out into a single method:

So let’s try it again:

@app.route('/neighbors')
def neighbors():
    characters = self.get_orm().model.table('Characters')
    neighbors = characters.visible_to( self.character )
    return render_template('people.html', people=neighbors)
Enter fullscreen mode Exit fullscreen mode

Ah, that’s better. Except we have exposed our ORM (and thus, our database). We have many data sources which have data in Redis, or configuration files, or a custom cache, or from all sorts of locations that the controller should never, ever know about. Some of my clients with this anti-pattern have worked around this by trying to implant those other data sources directly in their ORM. This simply shuffles the coupling around. The controller should never care about the source of data. The controller should only be asking for the data. Instead:

@app.route('/neighbors')
def neighbors():
    characters = self.model('Characters')
    neighbors = characters.visible_to(self.character)
    return self.template('people', people=neighbors)
Enter fullscreen mode Exit fullscreen mode

Where do the “visible” characters come from? You don’t care. By hiding all of these details from the controller, the developers are free to implement the fetching of this data any way they want, so long as they respect the contract originally specified when this method was created.

And we return a template via a name, but we don’t claim it’s HTML. The instance can figure out its calling context and render accordingly.

The model is your business logic, not your data model. And I’ll go a step further and say a good design choice is to make the model provide services the client needs, with a headless application underneath. By creating a this, you can layer a controller and HTML view on it. If you’re careful, you can have your controller target multiple views and build a native mobile application on top of it, only requiring a new view. You could create a native client/server desktop application, too. If the controllers are too inflexible, that’s OK. They’re tiny and easy to skip. Write thin controllers specifically for your mobile and desktop apps, secure in the knowledge that the application logic is all in your headless application.

MVC controllers should be like air traffic controllers: they don’t fly the plane, they just tell them when to takeoff and land. When you write controllers without logic, that are only able to connect the view to the model, you’re writing better software.

Top comments (14)

Collapse
 
bbrtj profile image
bbrtj

Model is usually (in most frameworks) connected to database with some ORM. There were some frameworks which also used models for different stuff like form models. I actually liked that.
However, if your model is connected to the database, I wouldn't put any business logic into it, other than getting even more data from the database. ORM objects are rather hard to instantiate in tests and unstable to pass around in your application. I don't ever feel safe when my object has the ->save method.

  • you don't ever know what is going to save it and where
  • as mentioned earlier, creating a new instance of that model (not to get it into the database, just to have something to work on) gets an additional mental overhead
  • often harder to debug, as these objects contain much more than just the data you need
  • and they are actually coupled with the framework

So what I've been trying to do lately instead is to create DTOs (Data Transfer Objects) for pretty much everything and repositories to save and fetch DTOs. Models are just used by the repositories. It is much easier in Perl because with Moose MOP you can skip tedious creation of DTOs and methods to translate DTO to model (usually two per model).

DTOs are easy to create, safe to pass around and in my case, they already contain all the right type constraints (with Type::Tiny). I also created a common DTO method named ->dummy which creates a new offspring class for the DTO with all the same type constraints but with no required fields, so that I can create DTOs even if I don't technically have all the data it needs. I admit that it's not a tested solution but works pretty well where I use it, and when I talk with my coworkers about it (in PHP context) they often agree.

Let me know if this is interesting to you, I can extend that to a full dev.to post with some Perl examples and such

Collapse
 
ovid profile image
Ovid

I think seeing a dev.to article about that would be great!

Collapse
 
brianwisti profile image
Brian Wisti

Even though Python can't legibly get the terseness of a Perl or Ruby, I still aim for the shortest clearest function / method that gets the job done.

I often start sloppy, putting the logic where I think of it, then refactoring out as I see repetition or big chunky complexity that obscures the job of the current function. Pretty remarkable how often those refactored bits end up being useful for other contexts within the application.

Collapse
 
ovid profile image
Ovid

I can understand that point of view and I used to be there. Over time, I discovered that when I knew that a particular place was bad for my code, leaving it there would cause issues in the long run. Refactoring a large, working application is hard work.

That being said, just because I know where something doesn't belong doesn't always mean I know where it does belong. Software is still hard 😃

Collapse
 
brianwisti profile image
Brian Wisti

Well, yeah. There is that. "Where I think of it" isn't quite as random as it was for me say ten or twenty years ago. Plus, the "refactor" flags start waving about five to ten lines in. The river follows the path of least resistance, but often enough it follows the path that already worked.

OTOH it's not uncommon for me to wake up to an ugly chunk of code I added last night at 2am that urgently needs to be cleaned up for my own sanity.

Collapse
 
smonff profile image
🌌 Sébastien Feugère ☔

How can we change the fact that people who implement those bad practices sometimes just follow the recommendations of doing the work the fastest and cheapest way, imposed by decision makers who don't know what they are doing, without thinking of the future, that will surely be a mess if done that way? 😭

Collapse
 
ovid profile image
Ovid

We'll never fully eliminate that because sometimes, doing things the fastest, cheapest way is what's needed to stretch the budget towards hitting that MVP. It's not an either/or proposition.

However, it can be mitigated by strong hiring practices up front. Hiring is one of the things tech companies are worst at (they're getting better a hard skills, but not soft skills). But if you get the right devvelopers, their default position is doing good work and cutting corners as needed. When the default is cutting corners, doing good work becomes much more expensive and much less obvious.

Collapse
 
ninofiliu profile image
Nino Filiu

Thanks for putting into words what I've had in mind for months!

In the same line of thought, I once had a senior dev tell us

The web UI and the database access should be implementation details

and as a dev whose work for the past years has basically been connecting web UIs to databases I took it very personally because I was like "so, what's left? :'("

Only with experience I later realised how important it was to have a Model that is agnostic to what happens outside of the business logic, that doesn't do database calls, and that doesn't generate any HTML

Collapse
 
ovid profile image
Ovid

It's a sad problem that for many companies today, a "senior" dev is decided by years of experience, not ability. Thus, technical ability is sometimes dismissed by the business side.

That being said, "years of experience" often means "business knowledge" and that's something which is sometimes dismissed by the technical side.

I've given talks before about how to bridge this divide, but it's hard to do. So many have chosen which side of the management/technology divide they're on and they're happy to stay there.

Collapse
 
jj profile image
Juan Julián Merelo Guervós

In general, tutorials are broken, at least from the student point of view. They show a single solution to a problem that may have different trade-offs, which are swept under the rug. They put the student into the mentality of "this is how it's done". That's rarely the case in a complex business environment.
Not to mention the speed with which they become obsolete, or how they introduce bad practices on the emitting side, and cargo cult programming on the receiving side.

Collapse
 
systemz profile image
Zahir Lalani

Separation of concerns is such a key aspect that many dont get

Collapse
 
randalschwartz profile image
Randal L. Schwartz

My brief way of saying this is "if your tests are hard (or impossible) to write, you probably need a refactoring or two or three..."

Collapse
 
ovid profile image
Ovid

Absolutely! I should write a post about that: tests that are hard to write are often a code smell.

Collapse
 
moopet profile image
Ben Sinclair

In particular, they had a 3,000 line “method” for their search engine.

Those are rookie numbers