meticulousengineer

You should not look at implementation to find out what the code does

Shall you check the implementation to find out what some code does? The answer is NO, even though there are some exceptions.

Designing APIs is not easy and requires years of practice to do it well.

Why is it not a good idea to rely on looking at implementation? Here are some reasons:

  • sometimes, the implementation is not available, especially when we talk about external systems or code provided runtime
  • understanding code, especially others’ code, is a complicated and time-consuming thing
  • essential details can easily be omitted while understanding code if they are not clearly stated

When we want to use a class or a function, we should check that function’s documentation. That’s why we created tools to generate documentation. That’s why people and companies pay money to have documentation. All engineers, not just from our domain, rely heavily on docs. Before using an integrated circuit, an electronics engineer will always consult the circuit manual to find out how the circuit should be connected, what the operating limits are, and what the inputs and outputs are. Not doing so could eventually result in destroying the whole hardware.

Since documentation is essential, it is not always required in the form we know it. When it comes to code, the best documentation is the API itself.

Let’s consider an example in which we call an external system using FeignClient to fetch a user by ID over HTTP, and the external system returns 404 NOT FOUND if the user is not identified:

class UserService {
  fun getUser(userId: UUID): User {
    // call external system
  }
}

There are many systems out there running similar code production these days.

The problem with this code is that it doesn’t tell you what happens if the user is not found.

One way to solve it would be to document it as follows:

class UserService {
  /**
   * @throws feign.FeignException.NotFound if user is not found
   */
  fun getUser(userId: UUID): User {
    // call external system
  }
}

This snippet has additional issues:

  • in case the user is not found, the NotFound exception doesn’t contain information about the user
  • since NotFound is a RuntimeException (unchecked exception), all callers will need to inherit the @throws docs

To fix those, we might end up handling the NotFound exception, wrapping it into another custom exception and throwing it:

class UserNotFoundException(val userId: UUID) : RuntimeException("User cannot be identified by id=$userId")

class UserService {
  /**
   * @throws UserNotFoundException if user is not found
   */
  fun getUser(userId: UUID): User {
    try {
      // call external system
    } catch (e: FeignClientException.NotFound) {
      throw UserNotFoundException(userId)
    }
  }
}

class OrderService(
  private val userService: UserService,
) {
  /**
   * @throws UserNotFoundException if user is not found
   */
  fun placeOrder(..., userId: UUID) {
    val user = userService.getUser(userId)
  }
}

The problem looks fixed in UserService, but now we have even more issues to resolve in OrderService. First, when we think about placing an order, we probably won’t expect it to do a user fetching. It’s not natural for this to come up in the reasoning process. Second, all callers of the OrderService should also add UserNotFoundException to their docs. This is how we will end up with documentation chaos, which is neither feasible nor productive even though we do it every day. In practice, it is very annoying to find out all usages of a method and add a @throws to each of them.

A better approach to solve this would be to rely on language features, tools and concepts, like Kotlin, ArrowKt and Either:

class UserService {
  fun getUser(userId: UUID): Either<GetUserError, User> {
    return try {
      // call external system
      result.right()
    } catch (e: FeignClientException.NotFound) {
      GetUserError.UserNotFound(userId).left()
    }
  }

  sealed interface GetUserError {
    data class UserNotFound(val userId: UUID): GetUserError
  }
}

Having this approach has a few advantages:

  • all callers of getUser() will be forced by the compiler to handle UserNotFound or to pass to their callers
  • the error is part of the method signature, which removes the need to add additional documentation
  • the callers of callers of getUser() will also be enforced by the compiler to handle UserNotFound

This approach in Java will not work well because Java still needs some essential features like exhaustiveness over sealed interfaces or sealed classes. Instead, a similar result can be achieved using checked exceptions.

I won’t make any recommendations for Java since documenting with @throws and the checked exceptions are terrible choices. But if I were to choose between one of those considering where Java is today, I would go with checked exceptions to have the compiler guarantee that all errors will be handled.

This code section is here to prove that we can reduce our cognitive and physical efforts by building well-designed APIs.

Imagine yourself in a big codebase where the code relies upon RuntimeExceptions, and there are no @throws in the function docs. If you are to implement a new feature, you will always end up scanning the entire codebase for thrown exceptions to find out what could potentially go wrong. And, practically speaking, the chances to spot all of them are minimal.

Here are a few questions that I believe good API docs should answer:

  • What is a class/function purpose?
  • Why is the class/function needed
  • What problem the class/function solves?
  • How the class/function works?
  • How the class/function should be used?
  • What are the constraints or limits of the class/function?
  • What are the constraints of the parameters of a function?
  • What could go wrong with the functions?
  • What errors might occur?
  • Why a specific implementation was chosen?

Any API should come packed with documentation. Its documentation should be clear enough to answer as many questions as possible. If something is missing from the docs, update it. Ask the caller to update the docs if it is not your code. If that’s not possible, make a note next to the calling place.

The last thing I would mention is to document only what is necessary. Nobody is interested in a setter comment that only puts the value in a value.

Posted

Tags:

,