Mark Methods Private When You Don’t Test Them

Written by: Pat Shaughnessy
4 min read
Stay connected

This article was originally published by Pat Shaughnessy on his blog, and with his permission, we are sharing it here for Codeship readers.

[caption id="attachment_1646" align="alignleft" width="350"]

My father in law once lived in same building where Picassowas born, near the Plaza de la Merced in Málaga, Spain.[/caption]

In Ruby and many other languages, you write private methods to implement internal logic you don’t want to expose. You want the freedom to rename, repurpose, or even delete them without worrying about impacting anything else.

The private keyword signals other developers: Don’t rely on this; don’t call it; it might change. This is especially important when writing framework or library code that many other developers will use.

But which methods should you make private? Sometimes this is obvious; sometimes it isn’t. A good rule of thumb to use is: If you’re not testing a method, it should be private.

But wait a minute! Aren’t we supposed to test everything? Isn’t 100 percent code coverage the nirvana every Ruby developer seeks? Let me clarify. You should mark methods private when you test them indirectly by calling the other, public methods in the same class. Use the private keyword to help organize your code, to remind yourself what you still need to test, and what you don’t.

Three Paintings

A simple example will make this clear. Suppose I have a class that describes a painting:

Now I can create a list of three paintings in a Minitest::Spec file like this:

Suppose my first requirement is to return the first painting from the list. Simple enough:

I just call Array#first, and I’m done. Returning the rest of the list is slightly more interesting:

Using a trick I learned from Avdi, rest always returns an array even if the input list was empty or had only one element. So far, so good. I’ve written two methods and two tests:

A New Requirement

Now suppose my business requirement changes slightly, and I instead need to return the first painting sorted alphabetically by name. Once again, it’s not hard to do.

And I need rest to use the same sort order, so I repeat the call to sort:

I’ve implemented new behavior, but still have two methods and two tests:

Extracting a Method

Because both of my methods are covered by tests, I’m free to refactor them. I decide to extract a new method, sorted_by_name:

Here I’ve simply moved the call to sort into a utility method called sorted_by_name. Now first and rest both call sorted_by_name, making the code a bit clearer and DRY-er. But now I have three methods and only two tests:

Mark Methods Private when You Don't Test Them

Notice I didn’t bother writing a test for sorted_by_name. I know it works because my other tests still pass. The existing tests are sufficient; I am testing sorted_by_name indirectly. Because I extracted sorted_by_name from first and rest, because I refactored my code without adding any new behavior, no new tests were required.

In this scenario, take the time to mark the new, untested method as private:

The private keyword here reminds me I’ve already tested sorted_by_name, that I don’t need to write new tests for it. Now private is helping me organize my code; it’s helping me remember which methods I don’t need to test… and which methods are missing important tests.

If my tests don’t need to know about sorted_by_name, then certainly other developers don’t. It should be private. Marking it private reminds me that it is being tested indirectly, that I didn’t just forget to write a test for it. Marking it private tells other developers about what I’ve learned from my own test suite.

Stay up to date

We'll never share your email address and you can opt out at any time, we promise.