Category: Software Design

Suppose we’re writing code that calls some external API, and this external API returns a JSON payload. When we call this API, there’s some nonzero chance that it will return invalid JSON (ie. a malformed response).¹ In the case when we receive a malformed response, we want to raise an exception back up the call stack. The code may look something like so:

Suppose that we run this code in production, and while it works for the happy path where SomeAPIClient returns valid JSON, we find some issue with the external API we’re unable to fix. For example, we may find that due to a bug in the API endpoint that SomeAPIClient calls, SomeAPIClient returns a string called "bar" (ie. a malformed JSON payload) for some fraction of the calls where parameters_hash has a key of :foo.

In this case, we treat this error as a known issue that we’re unable to solve, and so we want to write a workaround to gracefully handle this error. To write a workaround, we need error-handling logic that’s more specific than the default behaviour of raising a JSON::ParserError. We might be tempted to write code that looks like this²:

By raising a InvalidResponseForFoo when we see the malformed response of "bar", the above code handles the known issue with more specificity than simply raising a JSON::ParserError.³ However, the above code is insufficiently specific, and it may mask unrelated bugs that are introduced into our system in the future.

For example, what would happen if SomeAPIClient started returning "bar" intermittently for every value of parameters_hash, not just requests containing a key of :foo? Our application would raise InvalidResponseForFoo errors for requests completely unrelated to the key of :foo, meaning our workaround is executed for a use case it was not designed for.

In this example, a better way to handle the error would be like so:

In the revised error handling code above, we explicitly check if the key :foo exists in our input parameter. This ensures that we’re only raising a InvalidResponseForFoo for the known issue we originally observed.

This example is specific to JSON, but there’s a generalized takeaway from this: if we’re writing workaround code to deal with a known issue, we should be as specific as possible when determining whether or not to execute that workaround. This prevents us from masking future bugs that may look similar to to our known issue, but could be a different issue altogether.

Start your journey towards writing better software, and watch this space for new content.

1: Hint: we should assume this is true for every API call that we make!

2: In this example, we raise a custom error to handle this known issue. However, this isn’t the only way to handle a known issue. For example, we may want to return nil or implement some use-case specific logic.

3: A more efficient approach would be to write raise InvalidResponseForFoo if raw_response == 'bar' before we call JSON.parse(raw_response). This would allow us to only raise one error, and avoid writing an explicit rescue block for JSON::ParserError. However, I found the example to be a bit more readable with the error handling code written at the end of the method definition.

The Law of Demeter is a design principle used to develop well-encapsulated software. To hide implementation details between classes, the Law of Demeter suggests the following guidelines (taken from the linked Wikipedia article):

• Each unit should have only limited knowledge about other units: only units “closely” related to the current unit.

• Each unit should only talk to its friends; don’t talk to strangers.

• Only talk to your immediate friends.

For example, consider following classes A, B, and C:

If we wanted to retrieve the string 'foo' from an instance of A, the following code snippet would break the Law of Demeter:

a = A.new
bar = a.b.c.foo

By accessing instances of B and C via a, we’re exposing internal implementation details of the class A, and so this program has violated the Law of Demeter. A more encapsulated design defines our classes¹ like so:

With our new design, we can retrieve 'foo' from an instance of A like so:

a = A.new
bar = a.foo

The above code snippet can retrieve the string 'foo' from an instance of A without knowing the internal implementation details of A (namely, that it refers to B and C when retrieving that string).

The Law of Demeter is a valuable design principle, but for simplicity and conciseness, it’s sometimes stated as “use only one dot.” If we see multiple dots when reading a value, it suggests that we’re accessing some object’s internal implementation details. a.b.c.foo has three dots (ie. bad encapsulation), while a.foo only has one (ie. good encapsulation), so the heuristic works for the above example.

However, dot counting is just a heuristic, and one that we can’t apply blindly. For example, what if we called the following code on an instance of C from the previous code snippet?

c = C.new
bar = c.foo.upcase.reverse # has value of 'OOF'

c.foo.upcase.reverse has the same number of dots as a.b.c.foo – however, does it violate the Law of Demeter?

No, not at all. c.foo returns a string, a built-in Ruby type. .upcase and .reverse are methods on that built-in type which also return strings. There’s no encapsulation that’s being broken in this example, since we’re just doing transformations on a string.

A better way to apply the “dot counting” heuristic is to:

• check if there are multiple dots when reading a value
• if it’s greater than one, check how many (non built-in) different classes are being accessed after each subsequent dot
• if that number is greater than one, there may be a violation of the Law of Demeter

Heuristics are useful, but context matters when applying them.

Start your journey towards writing better software, and watch this space for new content.

1: For the sake of clarity, I wrote a foo method on each class in this example. However, Ruby on Rails’ delegate feature is a more concise and idiomatic way to achieve the same functionality.