Clean Code

What a rock-star
programmer is?

For me, a rock-star
programmer is:

Care for the
code you write

Avoid

"Broken Windows"

Use "The Boy Scout Rule":

Leave the campground 

cleaner than you found it

"Any fool can write code that a computer can understand. Good programmers write code that humans can understand"

- Martin Fowler

What is Clean Code?

Why is Clean Code so important?

If anyone can understand the code, then:

"The ratio of time spent reading (code) versus writing is well over 10 to 1 ... (therefore) making it easy to read makes it easier to write."

- Robert C. Martin

Why some people are
adverse to some of
the ideas of Clean Code?

Why is so bad
to have a lot of methods?

Because it once had
an impact on performance.

"Readability trumps performance" 

- Martin Odersky

"Premature optimization
is the root of all evil" 

- Donald Knuth

The key for improving
the performance
of an application is:

Measure, measure, measure.

Comments

"Good code is its own best documentation. As you’re about to add a comment, ask yourself, ‘How can I improve the code so that this comment isn’t needed?"

- Steve McConnell

Use comments judiciously, because they get easily outdated, leading to misinformation.

If your code can only be  understood with comments, try to rewrite it.

Suppose we have

public interface PrimeChecker {
  booolean isPrime(int n);
}

So we want to create an implementation using a Sieve

public class PrimeCheckerUsingASieve implements PrimeChecker { }

Is this Clean Code?

public class PrimeCheckerUsingASieve implements PrimeChecker {
  
  public static final int MAX_PRIME = 1000000;
  private boolean[] sieve;

  public boolean isPrime(int n) {
    return sieve[n];
  }

  public PrimeCheckerUsingASieve() {
    this.sieve = new boolean[MAX_PRIME + 1];
    Arrays.fill(sieve, true);
    sieve[0] = false;
    sieve[1] = false;
    for (int i = 0; i * i <= MAX_PRIME; ++i) {
      if (sieve[i]) {
        for (int j = i * i; j <= MAX_PRIME; j += i) {
          sieve[j] = false;
        }
      }
    }
  }

}

Can we make it
easier to understand
without comments?

Is this Clean Code?

public void init() {
  // code
  Arrays.fill(sieve, true);
  // more code
}

Or is this cleaner?

private void uncrossAllNumbers() {
  Arrays.fill(sieve, true);
}

public void init() {
  // code
  uncrossAllNumbers();
  // more code
}

Is this Clean Code?

public void init() {
  // code
  sieve[0] = false;
  sieve[1] = false;
  // more code
}

Or is this cleaner?

private void cross(int n) {
  sieve[n] = false;
}

private void crossObviousNumbers() {
  cross(0);
  cross(1);
}

public void init() {
  // code
  crossObviousNumbers();
  // more code
}

Is this Clean Code?

public void init() {
  // code
  for (int i = 0; i * i <= MAX_PRIME; ++i) {
    // more code
  }
}

Or is this cleaner?

private void hasAtLeastOnePossibleUncrossedMultiple(int n) {
  return n * n <= MAX_PRIME;
}

public void init() {
  // code
  for (int n = 0; hasAtLeastOnePossibleUncrossedMultiple(n); ++n) {
    // more code
  }
}

Is this Clean Code?

public void init() {
  // code
    if (sieve[i]) { /* more code */ }
  // even more code
}

Or is this cleaner?

public boolean isPrime(int n) {
  return sieve[i];
}

public void init() {
  // code
    if (isPrime(n)) { /* more code */ }
  // even more code
}

Is this Clean Code?

public void init() {
  // code
      for (int j = i * i; j <= MAX_PRIME; j += i) {
        sieve[j] = false;
      }
  // even more code
}

Or is this cleaner?

private void crossMultiplesOf(int n) {
  for (int x = firstPossiblyUncrossedMultipleOf(n); j <= MAX_PRIME; 
      x += n) {
    cross(x);
  }
}

public void init() {
  // code
    crossMultiplesOf(n);
  // even more code
}

Is this Clean Code?

public class PrimeCheckerUsingASieve implements PrimeChecker {
  
  public static final int MAX_PRIME = 1000000;
  private boolean[] sieve;
  
  public PrimeChecker() {
    createSieve();
    uncrossAllNumbers();
    crossObviousNumbers();
    crossMultiplesOfKnownPrimes();
  }
  
  public boolean isPrime(int n) {
    return sieve[n];
  }
  private void createSieve() {
    this.sieve = new boolean[MAX_PRIME + 1];
  }
  private void uncrossAllNumbers() {
    Arrays.fill(sieve, true);
  }
  private void cross(int n) {
    sieve[n] = false;
  }

Is this Clean Code?

  private void crossObviousNumbers() {
    cross(0);
    cross(1);
  }
  private void crossMultiplesOfKnownPrimes() {
    for (int n = 0; hasAtLeastOneUncrossedMultiple(n); ++n) {
      if (isPrime(n)) {
        crossMultiplesOf(n);
      }
    }
  }
  private boolean hasAtLeastOneUncrossedMultiple(int n) {
    return firstUncrossedMultipleOf(n) <= MAX_PRIME;
  }
  private int firstUncrossedMultipleOf(int n) {
    return n * n; 
  }
  private void crossMultiplesOf(int n) {
   for (int j = firstUncrossedMultipleOf(n); j <= MAX_PRIME; j += n
   {
     cross(j);
   }
  }

}

Clean Comments

• The first line of document must be your code.

• But you still need Javadoc-like comments.

• Don't comment code that you don't want anymore
  ( we have version control systems now )

Names

Is this Clean Code?

int d;

Is this Clean Code?

int d; // time elapsed in days

This is Clean Code!

int timeElapsedInDays;

Use meaningful names

Is this Clean Code?

void copy(char[] a1, char[] a2) {
  // some code
}

This is Clean Code!

void copy(char[] source, char[] destination){
  // some code
}

Avoid using numerations (a1, a2, etc.)

Is this Clean Code?

Date genymdhms;

This is Clean Code!

Date generationTimestamp;

Use pronounceable names

Is this Clean Code?

public class A {
  private int e;
  // More code
}

Is this Clean Code?

for (int i = 0; i < 34; ++i) {
  s += (t[i] * 4) / 5;
}

This is Clean Code!

final int NUMBER_OF_TASKS = 34;
final int REAL_NUMBER_OF_DAYS = 4;
final int WORK_DAYS_ON_A_WEEK = 5;
for (int i = 0; i < NUMBER_OF_TASKS; ++i) {
  s += (t[i] * REAL_NUMBER_OF_DAYS) 
         / WORK_DAYS_ON_A_WEEK;
}

Is this Clean Code?

ISomething myVariable = new SomethingImpl();

This is Clean Code!

Something myVariable = new DefaultSomething();

• Why do you want people to know that they are using an Interface?
  
• Lose the "I" on the names of Interfaces
• Don't use "Impl" for Implementation, use a name to describe the implementation (i.e. HttpServlet)
  (If there is nothing outstanding about the implementation use: Base, Default, etc.)
  

This is Clean Code!

public DayDate getOffsetNearestDayOfWeek(final Day targetDay) {
  int offsetToThisWeeksTarget = targetDay.index / getDayOfWeek().index;
  int offsetToFutureTarget = (offsetToThisWeeksTarget + 7) % 7;
  int offsetToPreviousTarget = offsetToFutureTarget - 7;
  return offsetToPreviousTarget > 3 ? plusDays(offsetToPreviousTarget)
          : plusDays(offsetToFutureTarget);
}

Use intermediate variables to help document your code.

Methods

Methods should...

• ... not have side effects.
• ... be small (one-liner is not bad)
• ... do one thing.
• ... not break DRY (Don't Repeat Yourself )

Write methods that will
let you print on console:

***
***
***

  *
 ***
*****

  *
 ***
*****
 ***
  *

Is this Clean Code?

public static void square(int size) {
   for (int row = 1; row <= size; ++row) {
      for (int column = 1; column <= size; ++column) {
         System.out.print('*');
      }
      System.out.println();
   }
}

Is this Clean Code?

public static void pyramid(int size) {
  int totalSpaces = size - 1;
  int totalChars = 1;
  for (int row = 1; row <= size; ++row) {
    for (int space = 1; space <= totalSpaces; ++space) {
      System.out.print(' ');
    }
    for (int character = 1; character <= totalChars; ++character) {
      System.out.print('*');
    }
    System.out.println();
    --totalSpaces;
    totalCharacters += 2;
  }
}

Is this Clean Code?

public static void rhombus(int size) {
  pyramid(size);
  int totalSpaces = 0;
  int totalCharacters = 2 * size - 3;
  for (int row = 1; row <= size - 1; ++row) {
    for (int space = 1; space <= totalSpaces; ++space) {
      System.out.print(' ');
    }
    for (int c = 1; c <= totalCharacters; ++c) {
         System.out.print('*');
      }
      System.out.println();
      ++totalSpace;
      totalCharacters -= 2;
   }
}

What is wrong with
those methods?

This is Clean Code!

private static char CHARACTER = '*';
private static char SPACE = ' ';

This is Clean Code!

private static String makeString(int size, char character) {
  char[] cString = new char[size];
  Arrays.fill(cString, character);
  return new String(cString);
}

This is Clean Code!

public static void square(int size) {
  for (int row = 1; row <= size; ++row) {
    System.out.print(makeString(size, CARACTER));
    System.out.println();
  }
}

This is Clean Code!

public static void pyramid(int size) {
   int totalSpace = size - 1;
   int totalCharacters = 1;
   for (int row = 1; row <= size; ++row) {
      System.out.print(makeString(totalSpaces, SPACE));
      System.out.print(makeString(totalCharacters, CHARACTER));
      System.out.println();
      --totalSpaces;
      totalCharacters += 2;
   }
}

Is this Clean Code?

public static void rhombus(int size) {
   pyramid(size);
   int totalSpaces = 1;
   int totalCharacters = 2 * size - 3;
   for (int row = 1; row <= size; ++row) {
      System.out.print(makeString(totalSpaces, SPACE));
      System.out.print(crearString(totalCharacters, CHARACTER));
      System.out.println();
      ++totalSpaces;
      totalCharacters -= 2;
   }
}

We are now breaking
DRY again, time to refactor!

This is Clean Code!

private static String makeFigure(int spaces, int characters,
      int size, int modifierSpaces, int modifierCharacters) {
  StringBuilder sb = new StringBuilder();
  for (int row= 1; row <= size; ++row) {
    sb.append(makeString(spaces, SPACE))
	    .append(makeString(characters, CARACTER))
	    .append(String.format("%n"));
    spaces += modifierSpaces;
    characters += modifierCharacters;
  }
  return sb.toString();
}

This is Clean Code!

public static void square(int size) {
  System.out.print(makeFigure(0, size, size, 0, 0));
}

public static void pyramid(int size) {
  System.out.print(makeFigure(size - 1, 1, size, -1, 2));
}

public static void rhombus(int size) {
  pyramid(size);
  System.out.print(makeFigure(1, 2 * size - 3, size, 1, -2));
}

Good (short) methods usually
have no more than 5 lines!

Auxiliary methods sometimes need more...
but no more than 10!

If your method needs
more than two indent level,
it's probably doing
more than one thing!

Is this Clean Code?

public void foo(/* parameters */) {
  for (/* something */) {
    // code
  }
}

public void bar(/* parameters */) {
  if (/* something */) {
    // code
  } else {
    // code
  }
}

This is Clean Code!

public void foo(/* parameters */) {
  for (/* something */) {
    auxFoo(/* some params */);
  }
}

private void auxFoo(/* parameters */) {
  // code
}

public void barWhenIf(/* parameters */) {
  // code
}

public void barWhenElse(/* parameters */) {
  // code
}

This is Clean Code!

public void theOneThaCallsFoo(/* parameters */) {
  // code
  if (/* something */) {
    barWhenIf(/* arguments */);
  } else {
    barWhenElse(/* arguments */);
  }
}

In reality it usually looks like this

public void theOneThaCallsFoo(/* parameters */) {
  // code
  T t = /* something */ ? v1 : v2;
  bar(/* arguments that include t */);
}

Again: don't be afraid of
many small methods.

Is this Clean Code?

List<Purchase> findTransactionBy(String firstName, String lastName)

This is Clean Code!

List<Purchase> findPurchasesById(Person person)

and this is also!

List<Purchase> findPurchases(Criteria criteria)

• Avoid a lot of arguments because it makes
  your API harder to remember how to use.

• Encapsulate related arguments on objects.

Exception Handling

This is Clean Code!

public T foo() {
  try {
    // Code
  } catch(SomeException e) {
    // Exception handling code
  }
}

So why none of the code
we tend to write, looks like that?

Is this Clean Code?

public T foo() {
  Bar bar = null;
  try {
    bar = something();
    // Code
  } catch(SomeException e) {
    // Exception handling code
  } finally {
    if (bar != null) {
      try {
        bar.close();
      } catch(AnotherException e) {
        // Code
      }
    }
  }
}

If it's hard to read,
it's hard to understand; 

You should rewrite it!

This is Clean Code!

public T foo(String path) {
  try(BufferedReader br = new BufferedReader(new FileReader(path))) {
    // read from the file
  }
}

Is this Clean Code?

public T foo() {
  try {
    // Code
  } catch(SomeException e) {
    // Code
  }
  return bar;
}

Is this Clean Code?

public T foo() {
  try {
    // Code
  } catch(SomeException e) {
    // Code
  }

  try {
    // Code
  } catch(OtherException e) {
    // Code
  }
}

Is this Clean Code?

public T foo() {
  try {
    // Some code
    try {
      // Some code
    } catch(OtherException e) {
      // Some code
    }
  } catch(SomeException e) {
     // Some code
  }
}

Is this Clean Code?

public T foo() {
  try {
    // Some code
  } catch(SomeException e) {
    try {
      // Some code
    } catch(OtherException e) {
      // Some code
    }
  }
}

In this cases, try to think
about all the possible
paths of execution.

It's really easy to
miss something
and hide a bug.

The solution?
Stop breaking the rule
of "do one thing"

Exception Handling
is one thing and a
method should only
do one thing, right?

So move logic to
an auxiliary method
and do error handling
in the called method.

Is this Clean Code?

public void loadParameters() {
  Properties p = new Properties();
  InputStream is = null;
  try {
    File file = new File("config.properties");
    is = new FileInputStream(archivo);
  } catch (Exception e) {
  }
  try {
    propiedades.load(is);
  } catch (Exception e) {
  }
  String width = p.getProperty("width");
  String height = p.getProperty("height");
  this.imageWidth = Integer.parseInt(width);
  this.imageHeight = Integer.parseInt(height);
}

How do we clean it?

Do a clean version of the method, without any error handling,
just throw all possible exceptions
(but never throw Exception)

This is Clean Code!

public void loadParameters() throws FileNotFoundException, IOException {
  Properties p = new Properties();
  InputStream is = new FileInputStream(archivo);
  propiedades.load(is);
  String width = p.getProperty("width");
  String height = p.getProperty("height");
  this.imageWidth = Integer.parseInt(width);
  this.imageHeight = Integer.parseInt(height);
}

• How hard is for you to
  understand this code?
  
  
• How many possible paths
  of execution does it has? One!

Now propagate until
you can end the
application and create
a user-friendly message

It was

public class Main {
  public static void main(String... args) {
    ImageResize resizer = new ImageResize();
    resizer.execute();
  }
}

but now is

public class Main {
  public static void main(String... args) {
    try {
      ImageResize resizer = new ImageResize();
      resizer.execute();
    } catch (FileNotFoundException e) {
      System.err.println("Config file does not exists");
    } catch (IOException e) {
      System.err.println("Config file could not be read");    
    }
  }
}

Now just separate logic
from error handling!

This is Clean Code!

public class Main {
  
  public static void run() {
    ImageResize resizer = new ImageResize();
    resizer.execute();
  }

  public static void main(String... args) {
    try {
      run();
    } catch (FileNotFoundException e) {
      System.err.println("Config file does not exists");
    } catch (IOException e) {
      System.err.println("Config file could not be read");
    }
  }
}

Let's try it one more time

Is this Clean Code?

private void resize() throws Exception {
  files = getNombreArchivos();
  for (int i = 0; i < files.length; ++i) {
    String filename = reader.getPath() + files[i];
    BufferedImage image = ImageIO.read(new File(nombreArchivo));
    BufferedImage newImage = resizeImage(image);
    String newFilename = destination + files[i];
    ImageIO.write(newImage, "jpg", new File(newFilename);
  }
}

This is Clean Code!

private void resize() {
  files = getNombreArchivos();
  for (int i = 0; i < files.length; ++i) {
    handleImage(reader.getPath() + files[i]);
  }
}
private void handleImage(String filename) {
  try {
    doHandleImage(filename);
  } catch (IOException e) {
    System.out.println(String.format(
        "File %s wasn't processed", filename));
  }
}
private void doHandleImage(String filename) {
  BufferedImage image = ImageIO.read(new File(filename));
  BufferedImage newImage = resizeImage(image);
  String newFilename = destination + files[i];
  ImageIO.write(newImage, "jpg", new File(newFilename);
}

There are other naggy things
about that code, but
error handling is Clean.

Now you can refactor!

This is Clean Code!

public T foo(/* some arguments */) {
  try {
    return doFoo(/* some arguments */);
  } catch(AnException e | OtherException e1) {
    // handle
  } catch(SomeException e) {
    // handle
  }
}

private T doFoo(/* some argmuments */) throws AnException, OtherException, SomeException {
  // Clean Code!
  Bar bar = foo.do(); // throws AnException
  bar.baz(); // throws OtherException
  if (foo.isBlitz()) {
    bar.platz(); // throws SomeException
  }
  return bar.doo();
}

Three more things for Clean Code on exception handling:

Classes

Don't break SRP

(Single Responsibility Principle)

This is Clean Code!

public class ControllerA {
  
  private ServiceA service; // ServiceA is a Façade
  
  public ControllerA(ServiceA service) {
    this.service = service;
  }
  
  @RequestMapping("/foo")
  public String doSomething(Bar bar, Model model) {
    Foo foo = service.something(bar);
    model.addAttribute(foo);
    return "viewA";
  }
}

public class Fraction {
  private int numerator;
  private int denominator;
  private void normalize() {
    int GCF = gcf(numerator, denominator);
    numerator /= GCF;
    denominator /= GCF;
  }
  public Fraction(int numerator, int denominator) {
    this.numerator = numerator;
    this.denominator = denominator;
    normalize();
  }
  private static int gcf(int a, int b) {
    // Euclid's algorithm
    return b == 0 ? a : gcf(b, a % b);
  }
  private static int lcm(int a, int b) {
    return a / gcf(a, b) * b;
  }
  public Fraction plus(Fraction other) {
    final int LCM = lcm(this.denominator, other.denominator);
    int denominator = LCM;
    int numerator = LCM / this.denominator * this.numerator 
        + LCM / other.denominator * other.numerator;
    return new Fraction(numerator, denominator);
  }
}

This is Clean Code!

public class MathUtilities {
  private MathUtilities() {}
  public static int gcf(int a, int b) {
    return b == 0 ? a : gcf(b, a % b);
  }
  public static int lcm(int a, int b) {
    return a / gcf(a, b) * b;
  }
}

public class Fraction {
  // Code
  public Fraction plus(Fraction other) {
    int LCM = MathUtilities.lcm(this.denominator, other.denominator);
    int denominator = LCM;
    int numerator = LCM / this,.denominator * this.numerator 
        + LCM / other.denominator * other.numerator;
    return new Fraction(numerator, denominator);
  }
}

Methods of a class should have high cohesion.

Organize methods on a class, so that reading a class feels like reading a story.

Boundaries

Is this Clean Code?

import com.othercompany.foo.Foo;

/**
 * Critical class for project!
 */
public class Bar {
  
  // Third-party utilitary class
  private Foo foo;
  
  public T platz() {
    // Code
    foo.doSomething();
    // More code
  }
}

This is Clean Code!

public class Bar {
  private FooService foo;
  public Bar(FooService foo) {
    this.foo = foo;
  }
  public T platz() {
    // Code
    foo.doSomething();
    // More code
  }
}

public interface FooService {
  T doSomething();
}

import com.othercompany.foo.Foo;

public class OtherCompanyFoo implements FooService {
  private Foo foo = new Foo();
  public T doSomething() {
    foo.doSomething();
  }
}

Unit Testing

• There is just one valid excuse to not write unit tests: that you don't know how to write unit tests (so you should ask a coworker for help)
  
  
• Writing unit tests doesn't make you finish your work in twice the time. It's the other way around: not writing unit tests is doing half the work you are supposed to do

If you don't write them:

• How do you know your code behaves as expected?
• How do you or other coworkers will be able to refactor the application's code without introducing bugs?
• How can you know that a reported bug is really a bug? How would you know is fixed?
• How would you know if code from others breaks your code?

TDD Steps

Why TDD?

• If you are going to write unit tests, why don't you write them first?

Writing Unit Tests (and using TDD) is a skill. You have to exercise for it to become comfortable.