2014 Graduate Bootcamp: Clean Code

Nicer to work with.
Less likely to have bugs.
Most importantly: easy to change.

Don’t add something because it might come in handy!
Outside-in TDD helps.

Regexps are the same.
What if you find a bug? What if they’re miles apart? Can you guarantee you’ll change both?

Extract duplicate code so each piece of knowledge is represented in one place.
Give it a name that makes it clear what it does.
You could also argue that there’s still duplication: only the field changes.

Is this the same? No!
Different concepts – incidental duplication.
If you extracted the regexp and order ID format changed, you’d break EIN validation.

Three responsibilities.
If one changes, need to modify the class. Potential to break stuff.

Employee class is now only responsible for persistence.

Introduce separate classes for other responsibilities.

Sounds crazy, right?

Currently we want to log an event whenever an employee is added.
What if we also wanted to send an email to the payroll team, or notify some other system? We’s need to modify this class.
Same issue as before – changing a class risks introducing bugs.

Allow registration of any number of listeners…

…then notify them all.
No need to change the class.

Square is a subclass of Rectangle.
Seems legit – a square is a rectangle, after all.

No complaints from the compiler, because Square is a valid Rectangle.

Hmm.

Turns out in this case it’s not valid for Square to be a subclass of Rectangle.
In this specific case, if the classes were immutable there wouldn’t be a problem.

Do we really want to risk code related to sending newsletters accidentally giving someone a payrise?
A bit contrived, but if we expose all an object’s methods everywhere, it’s easy for functionality to creep into the wrong places.

Narrow interfaces.

Employee now implements both.

Methods can be restricted to just the relevant interface.

Not going to go into detail about exactly what this means, but just a simple example.

EmployeeRegistry (high level) depends on Database (low level and specific).
What if we want to change database, or use a fake for testing?

DbConnection is an abstraction.
We now inject the connection into the constructor, so it can be replaced with a different implementation without changing the registry class.

Intent: each piece of the system should do one thing only, and make it clear what that is.
Duplication: concepts as well as actual chunks of code.
Ordered, highest priority first.
Good names better than comments.

Originally for Ruby.
Maybe add a few lines for Java because it’s more verbose.

Transcript

1. https://www.flickr.com/photos/cavale/4044396178
2. https://www.flickr.com/photos/patrickwilken/6093755090 Why?
3. YAGNI
4. DRY https://www.flickr.com/photos/roome/515419918
5. public boolean hasValidSourceIp() { return sourceIp.matches( "\\A(?:\\d{1,3}\\.){3}\\d{1,3}\\Z" ); } public boolean hasValidDestinationIp() { return destinationIp.matches( "\\A(?:\\d{1,3}\\.){3}\\d{1,3}\\Z" ); }
6. public boolean hasValidSourceIp() { return sourceIp.matches( "\\A(?:\\d{1,3}\\.){3}\\d{1,3}\\Z" ); } public boolean hasValidDestinationIp() { return destinationIp.matches( "\\A(?:\\d{1,3}\\.){3}\\d{1,3}\\Z" ); }
7. private String IP_ADDR_PATTERN = "\\A(?:\\d{1,3}\\.){3}\\d{1,3}\\Z" public boolean hasValidSourceIp() { return sourceIp.matches(IP_ADDR_PATTERN); } public boolean hasValidDestinationIp() { return destinationIp.matches(IP_ADDR_PATTERN); }
8. public boolean hasValidEIN() { return ein.matches("\\A\\d{9}\\Z"); } public boolean hasValidOrderId() { return orderId.matches("\\A\\d{9}\\Z"); }
9. SOLID https://www.flickr.com/photos/hisgett/5555618000
10. SOLID Single responsibility principle “Each software module should have one – and only one – reason to change.”
11. SOLID Single responsibility principle public class Employee { public Money calculatePay() { } public void save() { } public String reportHours() { } }
12. SOLID Single responsibility principle public class Employee { public void save() { } }
13. SOLID Single responsibility principle public class Payslip { public Payslip(Employee employee) { } public Money calculatePay() { } }
14. SOLID Single responsibility principle public class Timesheet { public Timesheet(Employee employee) { } public String reportHours() { } }
15. SOLID Open/closed principle “You should be able to extend the behaviour of a system without having to modify that system.”
16. SOLID Open/closed principle public class EmployeeRepository { private Logger logger = new Logger(); public void addEmployee(Employee employee) { create(employee); logger.employee_added(employee); } }
17. SOLID Open/closed principle public class EmployeeRepository { private List<Listener> listeners = new ArrayList<Listener>(); public void addListener(Listener listener) { listeners.add(listener); } // ... }
18. SOLID Open/closed principle public class EmployeeRepository { // ... public void hire(Employee employee) { create(employee); for (Listener listener : listeners) { listener.employee_added(employee); } } }
19. SOLID Liskov substitution principle “Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.”
20. SOLID Liskov substitution principle public class Rectangle { public void setWidth(int width) { } public void setHeight(int height) { } } public class Square extends Rectangle { }
21. SOLID Liskov substitution principle public void stretch(Rectangle rectangle) { rectangle.setWidth(2 * rectangle.getWidth()); } Square square = Square.new(2); stretch(square); square.getHeight(); // 2 square.getWidth(); // 4
22. SOLID Liskov substitution principle public void stretch(Rectangle rectangle) { rectangle.setWidth(2 * rectangle.getWidth()); } Square square = Square.new(2); stretch(square); square.getHeight(); // 2 square.getWidth(); // 4
23. SOLID Liskov substitution principle public void stretch(Rectangle rectangle) { rectangle.setWidth(2 * rectangle.getWidth()); } Square square = Square.new(2); stretch(square); square.getHeight(); // 2 square.getWidth(); // 4
24. SOLID Liskov substitution principle public class Rectangle { public void setWidth(int width) { } public void setHeight(int height) { } } public class Square { public void setSize(int size) { } }
25. SOLID Interface segregation principle “Many client-specific interfaces are better than one general-purpose interface.”
26. SOLID Interface segregation principle public class Employee { public void givePayrise(Money amount) { } public String getEmailAddress() { } } public class Newsletter { public void send(List<Employee> recipients) { } }
27. SOLID Interface segregation principle public interface PaidEmployee { public void givePayrise(Money amount) { } public interface Contactable { public String getEmailAddress() { } public class Employee implements PaidEmployee, Contactable { }
28. SOLID Interface segregation principle public interface PaidEmployee { public void givePayrise(Money amount) { } public interface Contactable { public String getEmailAddress() { } public class Employee implements PaidEmployee, Contactable { }
29. SOLID Interface segregation principle public class Newsletter { public void send(List<Contactable> recipients) { } }
30. SOLID Dependency inversion principle “(1) High-level modules should not depend on low-level modules. Both should depend on abstractions.”
31. SOLID Dependency inversion principle “(2) Abstractions should not depend on details. Details should depend on abstractions.”
32. SOLID Dependency inversion principle public class EmployeeRegistry { public void save(Employee employee) { db = Database.getConnection(); db.save("employees", employee); } }
33. SOLID Dependency inversion principle public interface DbConnection; public class EmployeeRegistry { private dbConnection db; public EmployeeRegistry(DbConnection db) { this.db = db; } public void save(Employee employee) { db.save("employees", employee); } }
34. https://www.flickr.com/photos/mwichary/2473180880
35. Simple code:
36. Simple code: 1. Passes all its tests
37. Simple code: 1. Passes all its tests 2. Clearly expresses intent
38. Simple code: 1. Passes all its tests 2. Clearly expresses intent 3. Contains no duplication
39. Simple code: 1. Passes all its tests 2. Clearly expresses intent 3. Contains no duplication 4. Has no superfluous parts
40. Simple code: 1. Passes all its tests 2. Clearly expresses intent 3. Contains no duplication 4. Has no superfluous parts
41. Simple code: 1. Clearly expresses intent 2. Contains no duplication 3. Has no superfluous parts
42. Simple code: 1. Clearly expresses intent 2. Contains no duplication
43. Simple code: 1. Fix bad names 2. Remove duplication
44. https://www.flickr.com/photos/sd/3578648771 Sandi Metz’s rules
45. https://www.flickr.com/photos/sd/3578648771 Sandi Metz’s rules Classes can be no longer than one hundred lines of code.
46. https://www.flickr.com/photos/sd/3578648771 Sandi Metz’s rules Methods can be no longer than five lines of code.
47. https://www.flickr.com/photos/sd/3578648771 Sandi Metz’s rules Pass no more than four parameters into a method.