Code Quality Challenge — A diary

Daniel Zanzini
Goiabada
Published in
6 min readFeb 19, 2018

I've always tried, without success, to finish these X days challenges in doing something: 30 days writing challenge, 365 days photo challenge, 100 days JS programming challenges… And the list goes on. I don't have an exact reason for not ending, or even not starting, these challenges. I simply didn't.

But, a month ago, I started the 30-day code quality challenge, proposed by Ben Orenstein. This time, I finished it and want to share my experience with you.

Before we start

The 30-day code quality challenge is language-agnostic, but the project I'm working on is based on the Ruby on Rails framework. All code snippets used in here were gotten from this project.

That said, let's start.

Day 1 — Clean up your README

This was simple, but I had never written a README before. I've been working on the same project for four months and the README was blank… Until the day I accepted this challenge. In 20 minutes I had a simple but useful README that explains what the project is about and how to start the development for it.

Day 2 — Nuke TODO comments

I had luck with this one. I ran a search for TODO in my codebase and found only this one:

def handle_requisition_error(response)
# TODO: Do something here
context.fail!
end

Two days before this exercise, we had merged a commit that added Rollbar to our project so, instead of adding the TODO to an issue tracker, I decided to finish it.

It was fast and straightforward. It took half an hour and when I searched for TODO again, nothing appeared. Nice.

Day 3 — Get rid of a warning

I thought I would have nothing to do this day. I ran rails console,rails server, and rspec spec/ and no warnings. So I talked to one of my coworkers about it and he reminded me that bundle can output some warnings. I ran the command and got a nice warning telling me that I was using an old version of Bundler:

The nice warning with a (fragment of the) rebel symbol behind it :)

I don't know why I didn't update it before. Actually, I never read it, at all. I think I got used to ignoring warnings after working on a project that was full of them.

I just ran the command suggested by the warning itself and it was solved. It took me less than a minute of work.

Day 4 — Excise some unused code

The project I'm working on is a small one, created and maintained by myself. I always tried to check and remove all unused methods when refactoring things.

But, in the e-mail, Ben recommended the unused tool to check if you have code that is not being used. I decided to install it and give it a try. As a result, I got a green message saying: "Unused found no results" 🎉.

To check if the tool was really working, I ran it in an old project that I was sure there was unused code and I got a very big list. So, even if I didn't code anything in this day, I learned about a very useful tool to run in my next projects from time to time.

Day 5 — Trim those branches

The last project I worked on was a big one with a huge codebase. There I learned that keeping your codebase clean is very important to keep your sanity.

Therefore, since the beginning of this new project, I kept cleaning the merged branches so, when I ran git branch -r I just got branches that we're currently working on.

The result of git branch -r. Only 3 branches returned.

Day 8— Extract a compound conditional

Compound conditionals are statements composed of two or more conditionals. To find them I scanned my project for && and || and got one occurrence that was used in three distinct controllers:

The example I found on my project.

It was a compound conditional made of two conditionals. The idea was to check if we should render the form_from_another_user_error. I created a protected method inside ApplicationControllercalled should_render_form_from_another_user_error? and placed the logic inside it:

Still not enough. The reason why I should render the error is not clear. To make this right I needed one more change: extract each conditional to its own method.

The first one is checking if the form doesn't belong to the current user so a method called form_from_another_user?(form) should solve it. The second one checks if the action is not a transfer so I decided to call it not_a_transfer_action?:

The complete solution

Now, instead of having a compound conditional inside my controller I have one method with an explicit name composed by two methods with good names. No one will ever need to guess what this piece of code does. 💥

Day 9— Slim down an overgrown class

First of all, I needed to check what classes of my project were the longest ones. So I ran the command given on the task to check it: find ~/code/my_project -name "*.rb" | xargs wc -l | sort -rn | head and got this list in response:

The list of my longest files

The command returned only specs and I don't know if having big specs files is a problem. Is it?

As a fallback task I needed to improve something about a class of my project. I went through my project searching for some class that I could refactor and found one folder named services containing only one class named third_party.rb.

The purpose of this class was not clear by its name or folder. This class is responsible to handle API connections with this third party software by including HTTParty.

I improved this code by creating a new folder called apis, moving the ThirdParty class to this new folder and renaming it to ThirdPartyApi. By doing it I made the purpose of this class clearer than before.

Day 10— Create a bin/setup script

This day's task was to create a script to setup my app for development use. I picked Ben's example, opened the bin folder and… found out that Rails generates a setup script.

The setup script generated by Rails ❤

So, all I needed to do is to update my README.md to include how to use the script.

Day 11— Run your tests with your wifi off

No problems here. My tests ran fine without connection. The system uses a third-party API but it's already mocked in specs. I'll show you how I did it. First of all, let's take a look at the controller:

The action that connects to a third-party API

To perform this create action I need to connect to a third-party API. I put all the interaction with this API inside an interactor called PerformLogin. It gets the response from the API and I handle it on the controller based on the interactor response.

So, on my specs, all I needed to do is to mock the response from PerformLogin and all the interactions with the third-party API will not happen anymore:

The specs to mock the interaction with the outside world.

Is this the end?

No! For now, we're half of the way through these daily challenges.

Now continue on to Code Quality Challenge — A diary — Part 2!

Have any doubts?

Have any questions about the code quality challenge? Want to know more about any of these daily tasks? Have anything to share about code quality? Talk to us :)

--

--

Daniel Zanzini
Goiabada

Developer @ Guava Software. Passionate about Pão de Queijo. From Minas Gerais. Star Wars fan. Constantly happy, listening The Doors and drinking beer.