You don't have null checks as the beginning of a public method - reidev275/SignsYouShouldProbablyRefactor GitHub Wiki

Example


public string Trim(string s)
{
  return s.Trim();
}

class Server
{
 readonly IBar _bar;
 public Server(IBar bar)
 {
   _bar = bar;
 }

 public void DeliverDrink()
 {
   _bar.ServeDrink();
 }
}

interface IBar
{
 void ServeDrink();
}

Why is this bad?

  1. If we have a method that requires a parameter we're forced to use it by extension of the Law of Demeter. We should validate the parameter as soon as it comes under our control so that we have a central, common point for validation.
  2. If we don't validate bar in the Server constructor we'll be forced to validate it each time we want to call ServeDrink or risk a NullReferenceException being thrown.

Resolved

By performing a null check and throwing an exception as soon as we have control of the object we can protect Foo from throwing unintended exceptions. Either way an exception is thrown, but now we have explicitly defined what the exceptional case is and when the exception should be thrown which simplifies the callstack and also debugging.

class Server
{
 readonly IBar _bar;
 public Server(IBar bar)
 {
   if (bar == null) throw new ArgumentNullException("bar");
   _bar = bar;
 }

 public void BringDrink()
 {
   _bar.ServeDrink();
 }
}

interface IBar
{
 void ServeDrink();
}

public string Trim(string s)
{
  if (s == null) throw new ArgumentNullException("s");
  // or
  return (s ?? "").Trim();
}