This article was originally posted on our blog.
The majority of the Rails custom software development work we do at NextLink Labs deals with rehabilitating older projects that are overwhelmed with technical debt from years of development without refactoring.
The software problems in these websites are never introduced maliciously. Instead, these problems tend to build up gradually over months and years after many rushed deadlines, strict client demands that don't allow for refactoring, and inexperienced Rails developers trying their best but getting overwhelmed by "Rails magic".
After working on many of these kinds of projects, we've identified a number of categories for these kinds of problems which we outline below with possible fixes.
A lack of automated tests is by far the most common problem with Rails projects.
Typically this happens when a developer starts a new project and is writing all the code themselves. Because they're writing everything, they never feel a need to "waste time" writing automated tests.
Inevitably, the project grows large enough that manually testing features in the browser becomes a pain, and it becomes obvious a test suite is needed. However, by that point the amount of tests that would be needed for adequate coverage is overwhelming, and so the tests never get written.
The most important step to start fixing this problem is to write just one "sanity" test that checks the most basic pieces of your app. As soon as you have one test on a project, you have a basic testing foundation. Then when you have downtime on a project or when you're adding new features later on, you can slowly add more tests using that basic foundation you've set up.
Although we don't necessarily recommend all teams follow a strict Test-Driven Development (especially when a team is inexperienced with Rails testing), we do recommend at least writing a few high level integration tests before building any new feature. This ensures that even in the worst case scenario, you have at least one test that will check each feature's behavior is correct.
Developers that are new to testing will often swing the opposite direction after learning about the peace of mind a good test suite brings, and they will write dozens of redundant tests. This is an inevitable part of learning how to test Rails applications, and it's certainly a much better problem to have than too few tests. However, having a test suite that takes 15 or more minutes to run can throw off a team's momentum.
A slow test suite is most often caused by either an over-dependence on feature tests that use tools like Caybara to check the front-end or by having too much build up and tear down in your tests themselves.
To solve the problem of too many feature tests, first make sure you are trusting your unit tests and controller tests. This is a common problem where newer developers only trust a test that goes through a full feature the way a human would.
After you've removed any of these unit tests that are just disguising themselves as feature tests, consider combining multiple feature tests together. By their nature, feature tests require a lot of build up time to fill your database with the necessary records, so just combining 3-4 related tests into one longer test can save a lot of time.
For example, rather than having separate tests for searching, filtering, and sorting records, we create a single long test where a user creates a record, filters for that record, then deletes that record, etc. In this way, we limit each feature to one happy path test and several error path tests, and keep our Capybara feature test count very low compared to unit tests and controller tests.
We also make extensive use of RSpec's stubbing capabilities and use dependency injection wherever possible to keep build up and tear down speed fast. Dependency injection is a paradigm in software that states a class should mention no other classes by name and instead they should be passed in as parameters when instantiating an object. This allows us to pass in stubbed RSpec objects which are extremely fast and allow us to test the behavior we've isolated.
Creating "Fat Controllers" is an extremely common mistake for developers who are new to Rails. It's natural to want to keep your code all in one place, and it can take a while before you start to see the problems with having 20 or more lines of application logic inside controller methods.
Eventually, putting everything inside of a controller leads to unreadably long controller actions with many different instance variables. Because of how Rails controllers and views interact, it can be hard to tell if a specific instance variable is still being used when you have 5-10 variables declared inside a controller.
For this reason, we keep our controller actions to 5 lines or fewer and we only instantiate 1 instance variable in each controller action. To do this, we often create custom classes for a specific controller action, for example, an EventFinder class for a meetup style application or a Dashboard object for an application that has several different graphs or charts on its homepage. We find these custom classes much easier to deal with and much easier to test without going through the front-end of the application.
The most common solution to the "Fat Controller" problem is to create "Fat Models". We see many intermediate Rails developers follow the "Skinny Controller, Fat Model" paradigm, but this can lead to problems as well, especially if those models are inheriting from ActiveRecord and are not standalone Ruby classes.
When you shove all of your logic into ActiveRecord models, you start to get giant files that are over 200-300 lines long and unreadably complex. Your controllers will look much simpler, but the complexity is just one layer deeper. All your business logic is still being organized by your database tables and is being mixed in with all your persistence logic.
We advocate for using standalone Ruby classes, often referred to as service objects, for all of your application's business logic. These classes do not inherit from ActiveRecord and are kept in a separate folder, e.g.
/app/services. This allows you to keep your model files small and strictly related to the structure of your data, for example
belongs_to statements and any kinds of validation logic that may be needed.
ActiveRecord callbacks are a common tool used by Rails developers, but we've found they cause more headaches than any other aspect of "Rails magic" when we take on older projects. Callbacks like
before_action, etc. are often tempting to use because they can hide complexity and make code look appealing, but they also hide large amounts of implicit behavior that prevents new developers coming on to a project from understanding the application's flow and developing a mental model for how the codebase works.
We've seen countless bugs over the years from ActiveRecord callbacks, and at this point, we tend to avoid them at all costs. When we find them in client projects, we typically try and refactor them into explicit behavior whenever possible.
Besides just hiding complexity, another problem with callbacks like
before_action specifically, is that they're used by developers to DRY up code prematurely. Often times, code that is inside a
before_action callback will need to be changed slightly for each controller action as a feature is expanded. This turns the
before_action method from a simple 1 line ActiveRecord query into a 20 line conditional that is extremely hard to parse.
We're much more comfortable repeating the query inside each controller action, so if changes do need to be made to one specific controller action, there's no need for any messy conditionals.
As a project evolves and adds dependencies and setup, it's very easy for a README to become out of date. In our READMEs, we include information on how to setup the app, how to run tests and linters, how to deploy to staging and production, and information on any other scripts that need to be run regularly.
Ideally, all of this information should be updated when any of these processes change, but this can sometimes be forgotten. To ensure everything in the README is correct, whenever a new developer starts on one of our projects, we have them try to set up the project without interacting with other team members, only using the README. If they run into any problems or need to ask questions to a developer already on the project, we immediately update the README, and let them continue through the process until we confirm everything is correct. This gives us an easy way to check the documentation at least every few months.
We follow the 100 line file limit as a general rule of thumb in all of our projects, and one place where file length can quickly spiral out of control is with HTML views. All it takes is a few conditionals and a few sets of nested bootstrap divs, and a view becomes hard to understand without spending an hour or more doing line by line analysis.
To counter this, we use view partials whenever possible to logically break up long view files into easily understandable pieces. Sometimes developers new to Rails are hesitant to create views outside of the standard REST actions (show, index, new, etc.), but we've found separating views into custom folders and files makes our HTML much more understandable.
One important thing to keep in mind when using view partials is to never depend on a global instance variable inside the partial and always pass variables you need into the partial's render statement explicitly. Down the road, if you decide to reuse a partial inside a separate view, depending on an instance variable from a controller method can lead to both bugs from a missing variable or bad controller design by adding extra instance variables to a controller method.
A final anti-pattern I've noticed with new developers is a tendency to use gems instead of creating their own classes and database tables. I encountered an example of this recently where a junior developer was building a messaging feature between users. Their first instinct was to look for gems to implement this feature and they found the mailboxer gem. Mailboxer is a fairly complex gem which will build a full mailbox system (4 database tables with 10+ fields each) with features such as inbox, outbox, read receipts, etc.
These extra features were all overkill for the project we were building and would have added a ton of extra complexity to our small messaging feature. After talking through the problem together, we agreed on taking a simpler approach where we created our own Messages database table which had a sender and receiver and a single content field to store the message itself.
The junior developer mentioned not trusting their own ability to create a new database table, which is a common theme with newer developers. Adding a new gem to the Gemfile and running a generation script can seem like it's always a better option when you're inexperienced, but even if the gem is written by a more experienced developer, gems can introduce lots of unnecessary complexity which weigh down your app over time.
Technical debt is a growing concern in custom software development, especially for those that are taking over legacy projects. If that sounds familiar and you need some help, read our guide for successfully inheriting a legacy rails project here. If you need help getting started with a custom software development project or need help navigating the digital transformation, contact us.