Milestones and how CodeClimate helped us reach them

Goals are your lifeblood.

If you don't pick a destination, you'll never get there. Even if you have a map, you'll never find your way. You have no target.

A few months ago, we started using CodeClimate. We knew we had some bad code, and bad code comes in many forms.

Some code was just dusty. We hadn't touched it in over a year. As we all know, you learn a lot in a year. If you think code you wrote a year ago is still great, you probably haven't learned much.

Some code was poorly designed. It was a tightly coupled mess. We've written about this recently on how you should opt into your business logic. Tightly coupled pieces of code will cause you to pull out your hair. Avoid it at all costs. You should check out Ruby Science if you currently feel this pain. It's a great eBook written by thoughtbot which we used to further train ourselves into thinking about code in a better way.

CodeClimate helped us set the goal. It gave us a destination. When wet set our goal, our GPA was 2.5.

At first, we were slightly depressed that our code base received such a terrible score. A few bad classes and bad practices were pulling our GPA down. After all, our code base was almost two years old - and every code base incurs technical debt at some point. With client work, it was debt we couldn't always pay back.

When wet set our goal, our GPA was 2.5.

Don't be like us and get depressed. Many of our favorite code bases have relatively poor GPAs. ActiveMerchant is stuck at a 1.7 GPA right now (although this also includes many third party gateways). Discourse is a 2.8. Paperclip is a 3.1. Rails is a 3.4. Devise is a 3.5. Rails and Devise are pretty damn good.

These aren't amazing 4.0 GPAs like you might expect. Every code base has some parts that are complex.

We set a goal of 3.0. It was achievable in the short term (within a few months). It wasn't unrealistic (like 4.0).

To achieve that goal, we did a number of things:

  • We stopped the bleeding. We stopped allowing any questionable code into our code base. We were very strict with this. No longer were we shipping things and saying "we'll come back to this later."
  • We fixed broken windows. Anytime we were adding a new feature or fixing a bug, we'd refactor.
  • We started having weekly technical discussions with our engineering team. We'd talk about accomplishments over the past week and we'd prepare examples for all of us to talk about and work through.
  • We took some risks and aggressively refactored with proper unit tests. We took the time to extract important pieces of our application into smaller, thoroughly-tested units. This was risky in we were refactoring payment processing and handling of student credits.
  • We collected momentum while watching our GPA rise. We did not tolerate decreases in our GPA, only increases.

As it stands today, our GPA is 3.21 and rising.

We've eliminated numerous code smells. We've properly extracted business logic which transformed F classes into A classes. We have better test coverage for the important pieces of our application. The application is more stable overall. We're not afraid of breaking anything. Classes are better loosely coupled. We've eliminated most ActiveRecord callbacks (and I recommend you eliminate yours, too). Avoid callback hell.

As you can see in the charts above, we still have some work to do. There are still some hairy spots. We still have problems. We work everyday to make our code better as should you.

There's no reason to get depressed though. After all, the most important metric for your product is: Do your customers find it useful? These scores don't mean anything if your product sucks and nobody uses it.

However, improving your scores pay dividends down the road.

  • We're reducing complexity. It's much easier to jump into the code base and understand what's going on.
  • We solve bugs more quickly.
  • It's much easier to refactor for a number of reasons. We have simpler tests. We don't rely on the database much for testing since our business logic is composed of small, simple Ruby classes.
  • We can expand upon existing functionality easier. Things are no longer tightly coupled. If we want to reuse something, it's pretty easy to use it in another place.
  • Our developer happiness has increased. We don't have to work in a shitty code base anymore. Watching our GPA improve is a big boost to our confidence.

We've had some fun over the past few months improving our code. I hope you find this advice useful and start using parts of it in your own teams. Let us know how we can help.

CodeWars shows you there are so many ways to write code

As a team, we've been playing around with CodeWars.  CodeWars is a great way to learn the basics of writing code. They currently support Ruby, JavaScript, and CoffeeScript. We've had some fun with it - discussing the best solution to each problem as a team.

While CodeWars is fun, my favorite part is completing a solution and comparing your answer to the many other answers people have submitted. It's such a great way to learn and you remember where you've come from. You might tell yourself, "I remember when I would've written code like that" or "That's interesting. I never thought about it that way." 

This is completely different from StackOverflow. StackOverflow encourages one accepted answer. How often do you Google for a solution and simply look at the accepted answer? After you work the problem out yourself, CodeWars makes it really simple to scroll through other solutions by real people. 

Here's what I mean. I pulled a few examples for you. I've also given my reaction to each solution. 

Complete the keysAndValues function so that it takes in an object and returns the keys and values as separate arrays.

Simple sort, but this time sort regardless of upper / lower case.

That's why I love CodeWars. I'm not sure if signup is open to everyone, but if you need an invite, let me know. 

Opt-in to your business logic

  Thanks to Bruno for the section on inheritance.

In a legendary post by Bryan Helmkamp, he talks about seven ways you can refactor fat models. All seven relate to extracting code into separate plain old Ruby objects and using them when needed. And this past week, our team had a discussion about inheritance and how it's frowned upon by some of the experts in the Ruby and Rails fields. After really paying attention to all of this and trying to get to the essence of it, here's what I've learned.

The main idea is this: opt-in to your business logic

Opt-in is synonymous with a white-list. Instead of allowing everything and deciding what to exclude (black-listing), you white-list instead. You make a conscious decision as to what should be allowed. You use a white list when you use attr_accessible. You are allowing certain attributes to be mass assigned.

In the case of your business logic, you have to make a conscious decision as to when business logic should execute. 

If you want to save a model from the console, it shouldn't trigger 5 callbacks.

If you have a bunch of callbacks in your model and all of them include some sort of conditional, that's bad. If you want to do some backend work or a migration, you'll have to trace through your code to figure out if the callbacks will run. You have to ask the question, do I even want these callbacks to run? 

Updating a simple field like the name of something should be an easy task.

The exception to this rule is persistence. Callbacks can be managed nicely if you stick to only using them for persistence and avoid using conditionals. If you have a bunch of conditionals on your callbacks, it means they don't execute every time. If your callbacks don't execute every time, they aren't part of the core domain logic and shouldn't be in the model anyway. 

Here's an example of callbacks with conditionals. I'm sure you've seen some code like this before. To figure out which ones will run, you'd have to also look for and understand what the conditionals will return.

Your object should do one thing and do it right every time.

Granularity matters. If you want to opt-in to your business logic, you should be able to opt-in to any combination. 

For example, if you want to post to social networks after a model is created, you should be able to pick and choose which ones you want. If I want to post to Facebook, it also shouldn't post to Twitter.

Take a look at the example below. If you don't want to post to Twitter, you can simply comment out the TwitterPoster and you're done. It's a much better option to keep these separate rather than use a more generic SocialNetworkPoster..

Inheritance is very rigid, usually there is a better option.

If you're writing some code that will be used by a couple of classes in your app, use inheritance only if you absolutely must.

It is common knowledge among folks who practice object oriented design (not just Rubyists) that 'composition should be preferred over inheritance'. In other words, if you were thinking about making a superclass and subclass objects, don't - most likely you can achieve the same thing by making a Ruby module.

Here's a simple example that nicely shows the limitations of inheritance.

If you use these methods and other to opt-in to your business logic, you'll find yourself enjoying your code more. You'll be able to refactor things more easily. You won't find yourself in callback hell and you'll be able to write classes composed of many smaller units. 

Once a unit works, it should work forever. That's the idea. You'll find that you won't have much churn in your units which leads to better code.

 


An intro to CanCan and managing permissions, authorization

CanCan is our goto library for managing permissions in our applications. Like any library, you have to know how to use it. It comes with its own set of nuances and patterns. If you don't properly organize and manage it, you'll end up with a load of technical debt.

Before getting into details, you should understand a few key details about CanCan. These are details we've run into many times before and it always seems to trip up new people using the CanCan library. 

  1. check_authorization  If you're using CanCan, you'll want to be sure you are checking authorization. This happens in your controllers. Most of the time, it should be an exception if you skip authorization. Why would you define a bunch of permissions and then forget to authorize against them?
  2. load_and_authorize_resource:  Based on the permissions you've defined in your abilities file, CanCan can load and authorize your models in your controllers. For example, if you have a PostsController and you use the index action, CanCan will load all @posts which a user has access to. There are exceptions and loading your index action will fail if you use a block to define permissions.
  3. Ability precedence:  This is one of the most important gotchas and you may lose some time trying to debug this. Make sure you read about ability precedence and experiment with it. It's important to note that with CanCan, manage means any. If you can manage a post, you can perform any action on that post. The first permission you define takes precedent over any that follow.

Here's a block of code to help you understand the points above: 

With the introduction of strong_parameters in Rails 4 (also a great pattern you can use in your Rails 3 projects), you'll want to have a pattern for using it in unison with CanCan. CanCan doesn't natively support strong_parameters yet. CanCan attempts to load params before being filtered by strong_parameters; thus, strong_parameters raises an exception.

For us, we've started to use cancan_strong_parameters  which patches CanCan to allow strong_parameters. We'll try to update this post when CanCan begins to support strong_parameters.

How do you handle strong_parameters with CanCan? Let us know in the comments below. 

 

Do you write bad CSS?

CSS is so powerful because you can do so much with it. It's constantly improving. CSS3 is an amazing advancement for web developers. 

However, since you can write CSS however you'd like, a lot of people aren't very good at writing it - myself included. Over the past few months, I've challenged myself and our team to improve. Here's what I've learned: 

Don't style elements by ID

It's tempting. You can do it. But, you shouldn't. Avoid it at all costs. 

IDs are meant to be unique. If you style an ID tag, it means you're styling a unique element; therefore, you're not writing reusable CSS. Remember the DRY principle? Styling an ID is a blatant violation of the DRY principle. 

Avoid more than three levels of nesting

Deep nesting lends itself to CSS that isn't reusable. If you have more than three levels of nesting, you're probably styling elements based on their container. When you have many levels of nesting, the context of an element dictates its style. Most of the time deep nesting will cause problems.  Deep nesting should be the exception, and not a common pattern.

Naming is hard

Naming is notoriously difficult, and it's just as difficult while writing stylesheets. Try to describe the element you're styling in English. Don't skimp on naming in exchange for less typing, but don't use extremely long class names either. 

Do you want to understand your code in 6 months? Try hard to improve naming. 

Think of elements in terms of modules or components

If you start thinking in terms of modules and components, you'll start to write better CSS. Sometimes, context matters; and therefore, calls for you to do some nesting. More often than not, nesting is bad. 

Components should be reusable. If you want to put a component on a different page in a different context, it should still look the same. If it doesn't look the same, it's either a slight variation of the component or its a completely new component altogether. 

Split components into separate files

Have you ever worked on a project with one massive, ugly, disorganized stylesheet? I think we all have - and it's no fun. A good practice is organizing your stylesheets by component. Styling some tabs to be used throughout your application? Use tabs.css as a filename. What about a header component? Use header.css.

If you're worried about including many stylesheets in production, don't. You should be using stylesheet compilation. If you're using Rails, we have the asset pipeline to help us do this.  If you need a standalone solution, try YUI compressor.