Receive New Tutorials

Check My Code: Tips to Keep Ruby Code Smell-Free

– {{showDate(postTime)}}

check my code

In this tutorial, we will walk through a few common Ruby code smells and how to best deodorize them. Code smells are symptoms of a deeper problem, and by eliminating them, we can leave our code elegant and odor free!

Common code smells include long classes, large methods, dead code, and repetition. These issues are not only bad practice, but they indicate that there is probably a structural issue with our code. Code smells might not cause an error immediately, but they make code difficult to maintain and might cause errors for edge cases. Below, we cover a few different smells, including dispensable code, long conditionals, classes with attributes but no methods, poor variable naming, and over commenting.

Check My Code for Smells: Ruby edition

1. Dispensable Code: If ___ Return True, Else Return False

The pattern, “If __ return true, else return false” shows up again and again. Returning true or false from an if statement is redundant as we will see below. This pattern is an example of code that is dispensable and will make our program bloated and difficult to read. Below we discuss two different ways to code this pattern: one is long and redundant, and the other is concise and elegant. Check out the following method on the Animal class, which determines if an animal is_a_peacock?.

class Animal
	attr_reader :type
	
	def initialize(type)
		@type = type
	end

	def is_a_peacock?
		if type == "peacock"
			return true
		else
			return false
		end
	end
end

Do you smell it? The conditional type == “peacock” already returns a boolean, so we don’t need to include it in an if statement. This six-line phrase can and should be simplified into just one line. See below:

class Animal
	attr_reader :type
	
	def initialize(type)
		@type = type
	end

	def is_a_peacock?(animal)
		type == "peacock"
	end
end

Ahh, that’s better! We will keep this in mind whenever we find ourselves falling into the “if ___ return true, else return false” pattern. By removing dispensable code, we can make our code clean and easier to understand. Let’s move onto smellier smells!

2. Long Conditionals

Often times we need to check whether a variable is one of many options. There are a few different ways we can do this, but some are much more odorous than others. In a previous tutorial, 6 Best Practices Ruby Beginners Should Know, we walked through the example below and noted that the following case/when statement is much cleaner than using the more common if/elsif/else logic.

puts "What is your major?"
major = gets.chomp

case major
when "Biology"
	puts "Mmm the study of life itself!"
when "Computer Science"
	puts "I'm a computer!"
when "English"
	puts "No way! What's your favorite book?"
when "Math"
	puts "Sweet! I'm great with numbers!"
else
	puts "That's a cool major!"
end

However, this strategy is still long and wordy. We can improve on this even further!

puts "What is your major?"
major = gets.chomp

# Set default response
major_responses = Hash.new("That's a cool major!")

# Add other responses
major_responses["Biology"] = "Mmm the study of life itself!"
major_responses["Computer Science"] = "I'm a computer!"
major_responses["English"] = "No way! What's your favorite book?"
major_responses["Math"] = "Sweet! I'm great with numbers!"

puts major_responses[major]

By mapping majors to responses, we can simply hash the major input and get the appropriate response as the output. We set a default value that is returned if the major entered is not included in major_responses. After setting up our major/response mapper, we only need one line, which finds the response in constant time. Note that if we were trying to write a solution that optimizes for space complexity, this would not be an appropriate solution.

3. Class With Attributes but No Methods

As we know, classes are used to organize objects that behave similarly and have the same properties. But what if we have an item that has no methods or no actions? See below.

class Person

	attr_reader: :height, :hair_color, :dominant_hand, :iq, :astigmatic?

	def initialize(height, hair_color, dominant_hand, iq, astigmatic?)
		@height = height
		@hair_color = hair_color
		@dominant_hand = dominant_hand
		@iq = iq
		@astigmatic? = astigmatic?
	end

end

As we can see, a Person has many attributes. But in our example, a Person does not have any actions. For this reason, a class is not the best way to represent this object. While functionally, a Person class does the job, it is best practice to use a Ruby struct. Structs are used to create simple classes and can be initialized much more concisely. Below are two different examples of how to create a struct.

Struct.new("Person", :height, :hair_color, :dominant_hand, :iq, :astigmatic?)

Or,

Person = Struct.new(:height, :hair_color, :dominant_hand, :iq, :astigmatic?)

We can now create a person like so:

sally = Person.new(165, "red", :left, 180, true)

And Sally’s attributes can be accessed exactly like we access attributes for an instance of a class.

sally.height 			# => 165
sally.hair_color 		# => "red"
sally.dominant_hand		# => :left
sally.iq			# => 180
sally.astigmatic?		# => true

From now on, we can “de-smell” our code by using structs whenever we have an object that can be fully represented by only its attributes. As a side note, structs also come in handy when dealing with node-based data structures such as linked lists, trees, and graphs.

4. Variable Naming

Name your variables appropriately. We’ve heard it before, and we’ll hear it again! While a, b, x, and, y are easy to type out, going back and understanding your logic days later is a pain. Poor variable nomenclature is a symptom of unmaintainable code and should always be avoided. See below.

A = :fluorescent
B = :incandescent
C = :led

class Thing
	attr_reader :x
	
	def initialize(x)
		@x = x
	end

	def lightbulb
		if x == A || x == B || x == C
			puts "I am a lightbulb!!"
		else
			puts "I don't know what I am..."
		end
	end
end

This example reeks of smelly naming issues. What’s x? What’s A? Who knows? Let’s be explicit with how we name variables, methods, objects, etc. This will help other developers understand our code and make it easier for us to go back and debug, days or even years later. Below is a much more maintainable version of the example above.

class GarageItem
		attr_reader :type
	
		def initialize(type)
			@type = type
		end

	def am_i_a_lightbulb
		if type == :incandescent || type == :fluorescent || type == :led
			puts "I am a lightbulb!!"
		else
			puts "I don't know what I am."
		end
	end
end

Now our code almost reads like plain English which makes it easy to understand and maintain.

5. Indexing Into JSON Objects with Long Chains

Let’s say we are trying to access data deep inside a large JSON object that we have imported and parsed into a Ruby hash. This pattern is common when accessing APIs. We want to access some piece of data that is inside a hash, which is in an array, which is in a hash, which is in another hash. While it may be tempting to just keep indexing until we have arrived at our destination, this technique often leads to errors down the road. Let’s take a look at this example response from a sample API that returns information about geographic regions.

Response for California

{  "California" => 
	{"Abbreviation" => "CA", 
	 "population" => 39144818, 
	"senators" => [ 
		{ "name" =>"Barbara Boxer", "party" => "Democratic", "website" => "https://www.boxer.senate.gov/" }, 

		{"name => "Diane Feinstein", "party" => "Democratic", "website" => "http://www.feinstein.senate.gov/public/"}
		]
	 }
}

If we want to use this API to find the email address of one of our senators , we may be tempted to write a method like so:

def get_first_senator_email(response, state)
	response[state]["senators"].first["email"]
end

While this method will work for most of the responses, we must consider the edge cases that it will fail on. We can’t just assume every state will have senators and each of those senators will have emails. Check out this response from the same API for Washington DC.

Response for Washington DC

{  "Washington DC" => 
	{"Abbreviation" => "DC", 
	 "population" => 658893, 
	senators => []
	 }
}

Washington DC has no senators (taxation without representation!), so our previous method get_senator_emails will throw an error like this one : NoMethodError: undefined method '[]' for nil:NilClass. If we are not completely certain that our JSON object will have the same format for every possible response, it is dangerous to chain methods. Instead, we should write the get_senator_emails method like so:

def get_first_senator_email(response, state)
	senators = response[state]["senators"]	
	if senators.empty?
		raise ArgumentError, "No senator information available"
	else
		response[state]["senators"].first["email"]
	end
end

By checking the return value of senators before chaining another method onto it, we can provide the user with helpful error information. We can now be confident when we call .first["email"] that we will not get a NoMethodError. Every time we add another link to our chain, we have to be careful and check that the previous item will always be the expected return value.

6. Over and Under Commenting

The last code smell we cover in this tutorial is poor commenting. Comments are important, but we should make sure that they supplement our code in a way that is helpful, not repetitive. There is a fine line between comments that are useful and those that just obfuscate code. Below is a sample of a method that determines if an integer, n, is prime. One solution is over commented, and the other has one much more useful comment.

Over Commented

def is_prime?(n)
	# loop until i * i is greater than n
	while i * i < n
		# check to see if n is evenly divisible by integer i
		if n % i == 0 
			# return false if it is
			return false
		end
		i += 1
	end
	# return true if n % i == 0 is never true
	true
end

Helpfully Commented

def is_prime?(n)
	# Any factor greater than sqrt(n) has a corresponding factor less than 
	# sqrt(n), so once i >=sqrt(n) we have covered all cases
	while i * i < n
		if n % i == 0 
			return false
		end
		i += 1
	end
	true
end

As we can see, the first example’s comments just reiterate what the code is doing and doesn’t provide any extra information. The second sample has a single helpful comment, and is not redundant. One useful trick is to think of comments as an explanation of why we are doing something and the actual code as an explanation of how we are doing something. Typically, comments should not explain how something is happening, because that’s what the code does. As we deploy more and more code, it will become more clear when a comment is useful and when it is unnecessary.

Wrapping up

In this tutorial, we discussed a few common code smells and how to fix them. There are certain patterns that recur often, and by analyzing how to handle these situations in the most elegant way, we can significantly improve our code style. If we think critically about our code and always try to be more concise, more clear, and more modular we will become habitual smell-free coders!

Author’s Bio

check my code Hannah Squier is a self-taught software developer, with a background in GIS and civil engineering. As a UC Berkeley Engineering graduate and early startup employee, she has navigated many complex challenges with her technical know-how and perseverance. While preparing for her next adventure to become a full time software engineer, she writes tutorials to give back to the developer community. When she’s not coding, Hannah plays frisbee and thinks about how to make cities better places to live in. Get in touch at hannahsquier@gmail.com.