Clean Code

Code is all about communication and communication is the professional developer’s first order of business

Functions should be as small as possible. The blocks within “if”, “else”, “while” statements should be one line long (probably a function call). The indent level of a function should not be greater than one or two.

public static String renderPageWithSetupAndTeardowns(PageData pageData, boolean isSuite) throws Exception {
    if(isTestPage(pageData))
        includeSetupAndTeardownPages(pageData, isSuite);
    return pageData.getHtml();
}

Functions should do one thing and do it well —

1. If a function does some steps that is just 1 level below the stated name of function, then it is doing just one thing
2. Functions doing one thing cannot be reasonably divided into sections — declarations, initializations, implementation etc

public Money calculatePay(Employee e) throws InvalidEmployeeType {
    switch(e.type) {
        case COMMISIONED:
            return calculateCommissionedPay(e);
        case HOURLY:
            return calculateHourlyPay(e);
        case SALARIED:
            return calculateSalariedPay(e);
        default:
            throw new InvalidEmployeeType(e.type);
    }
}
public boolean isPayday(Employee e, Date date) throws InvalidEmployeeType {
    switch(e.type) {
        case COMMISSIONED:
            ....
    }
}
public boolean deliverPay(Employee e, Money pay) throws InvalidEmployeeType {
    switch(e.type) {
        case COMMISSIONED:
            ....
    }
}

All the above functions will have to be changed every time a new type of Employee is added.

Switch statement should only be tolerated if —

1. They appear only once
2. are used to create polymorphic objects
3. are hidden behind an inheritance

public abstract class Employee {
    public abstract boolean isPayday(Date date);
    public abstract Money calculatePay();
    public abstract void deliverPay(Money pay);
}
public interface EmployeeFactory {
    public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType;
}
public class EmployeeFactoryImpl implements EmployeeFactory {
    public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType {
        switch(r.type) {
            case COMMISIONED:
                return CommissionedEmployee(r);
            case HOURLY:
                return HourlyEmployee(r);
            case SALARIED:
                return SalariedEmployee(r);
            default:
                throw new InvalidEmployeeType(r.type);
        }
    }
}

The ideal no. of arguments for a function is zero, then one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) shouldn’t be used.

When a function needs 3 or more arguments, wrap it into a class of it’s own.

Circle makeCircle(double x, double y, double radius); ❌ 
Circle makeCircle(Point center, double radius); ✅ 

If your function must change the state of something, have it change the state of its owning object.

public void appendFooter(String report) ❌ 
report.appendFooter() ✅ 

Objects hide their data and expose functions that operate on them. Data structures expose their data and have no meaningful functions.

// Data Structure
public class Circle {
    public point center;
    public double radius;
}
// Object
public class Circle {
    private point center;
    private double radius;
    public double area() {
        return PI*radius*radius;
    }
}

Procedural vs OO programming —

// Procedural programming
public class Square {
    public Point topLeft;
    public double side;
}
public class Rectangle {
    public Point topLeft;
    public double height;
    public double width;
}
public class Circle {
    public Point center;
    public double radius;
}
public class Geometry {
    public final double PI = 3.14;
    public double area(Object shape) throws NoSuchShapeException {
        if(shape instanceof Square) {
            Square s = (Square)shape;
            return s.side*s.side;
        }
        else if(shape instanceof Rectangle) {
            Rectangle r = (Rectangle)shape;
            return r.height*r.width;
        }
        else if(shape instanceof Circle) {
            Circle c = (Circle)shape;
            return PI * c.radius * c.radius;
        }
        throw new NoSuchShapeException();
    }
    public double perimeter(Object shape) throws NoSuchShapeException {
        .....
    }
}

Now that if we add a new shape, we must change all the functions in Geometry class to deal with it. This violates Open-Close principle. The code should be Open for extension (new shapes can be added) but closed for modification (existing code should not change when new shape is added).

// Object Oriented
public class Square implements Shape {
    private Point topLeft;
    private double side;
    public double area() {
        return side*side;
    }
}
public class Rectangle implements Shape {
    private Point topLeft;
    private double height;
    private double width;
    public double area() {
        return height * width;
    }
}
public class Circle implements Shape {
    private Point center;
    private double radius;
    private final double PI = 3.14;
    public double area() {
        return PI * radius * radius;
    }
}

Note that in OO style, on adding a new Shape, the existing code does not change. Hence, it follows Open-Close principle.

A method f of a class C should only call the methods of —

1. C
2. An object created by f
3. An object passed as an argument to f
4. An object held in an instance variable of C

The method f should not invoke methods on objects that are returned by any other method of the class C.

Error Handling for third party APIs is a best done with wrappers around.

ACMEPort port = new ACMEPort(12);
try {
    port.open();
} catch (DeviceResponseException e) {
    reportPortError(e);
    logger.log("Device response exception", e);
} catch (UnlockException e) {
    reportPortError(e);
    logger.log("Unlock exception", e);
} catch (XYZError e) {
    reportPortError(e);
    logger.log("XYZ exception", e);
} finally {
    ....
}

We are roughly doing the same work regardless of the exceptions. We can simplify our code by wrapping the API —

LocalPort port = new LocalPort(12);
try {
    port.open();
} catch (PortDeviceFailure e) {
    reportError(e);
    logger.log(e.getMessage(), e);
} finally {
    ....
}
public class LocalPort {
    private ACMEPort innerPort;
    public LocalPort(int portNumber) {
        innerPort = new ACMEPort(portNumber);
    }
    public void open() {
        try {
            innerPort.open();
        } catch (DeviceResponseException e) {
            reportPortError(e);
            logger.log("Device response exception", e);
        } catch (UnlockException e) {
            reportPortError(e);
            logger.log("Unlock exception", e);
        } catch (XYZError e) {
            reportPortError(e);
            logger.log("XYZ exception", e);
        }
    }
}

Don’t return NULL. Don’t pass NULL into methods.

Test Driven Development (TDD) asks us to write Unit tests first, before we write production code. You may not write more production code than is sufficient to pass the currently failing test.

Classes should begin with a list of variables — public static constants, then private static variables, then private instance variables. We should avoid having public variables without a good reason.

In a class, identify all the responsibilities and then break each responsibility into a class of it’s own.

Dependency Injection. Software systems should separate the startup process — when objects are constructed and the dependencies are wired together — from the runtime logic that takes over after startup

public Service getService() {
    if(service == null)
        service = new MyServiceImpl(...)
    return service;
}

We have a hard-coded dependency on MyServiceImpl and everything its constructor requires. We can’t compile without resolving these dependencies, even if we never use an object of this type at runtime.

Also, testing is a problem since we cannot really mock MyServiceImpl.

DI lets us, move all the aspects of construction to a separate module and lets us design the rest of the system assuming that objects have been constructed and wired up appropriately.

In Spring, we can implement DI by creating Beans either explicitly or using annotations.

Explicit creation :

AppConfig.java: A simple POJO which holds all the configuration to bootstrap your application.

// Configuration annotation is used to indicate start 
// of configuration for our application
@Configuration
public class AppConfig {

    // This method returns an instance of the Bean.
    // This Bean is now registered inside of the spring
    // and is available for us to use inside of our spring
    // application
    // All these beans are singleton by default and will only
    // execute this method the first time it's called
    @Bean(name = "speakerService")
    public SpeakerService getSpeakerService() {
        return new SpeakerServiceImpl(getSpeakerRepository());
    }

    @Bean(name = "speakerRepository")
    public SpeakerRepository getSpeakerRepository() {
        return new HibernateSpeakerRepositoryImpl();
    }
}

Then, to use the AppConfig in our spring application,

public class Application {
    public static void main(String args[]) {
        // We pass our AppConfig class in
        // AnnotationConfigApplicationContext.
        // This loads spring and loads configuration files into
        // our application context.
        // When this line executes, it's going to create a basic
        // registry with the beans defined in AppConfig.java file
        ApplicationContext appContext = new AnnotationConfigApplicationContext(AppConfig.class);

        // Old way of calling it by creating an 
        // instance of the class:
        // SpeakerService service = new SpeakerServiceImpl();
        // But with spring, we call the bean
        SpeakerService service = appContext.getBean("speakerService", SpeakerService.class);

        System.out.println(service.findAll().get(0).getFirstName());
    }
}

Now to inject the Bean into another class, we can use @Autowire on that class setter method —

public class SpeakerServiceImpl implements SpeakerService {

    private SpeakerRepository repository;

    public SpeakerServiceImpl() {
        System.out.println("SpeakerServiceImpl no arg constructor");
    }

    @Override
    public List<Speaker> findAll() {
        return repository.findAll();
    }

    // This automatically injects the SpeakerRepository bean 
    // into this class
    @Autowired
    public void setRepository(SpeakerRepository repository) {
        this.repository = repository;
    }
}

Annotations :

AppConfig.java —

// Configuration annotation is used to indicate start
// of configuration for our application
@Configuration
// The text inside ComponentScan tells this is where
// I should begin looking for beans to autowire
@ComponentScan({"com.masne"})
public class AppConfig {
    // Nothing to be defined here
}

The service class —

// This creates a bean for SpeakerServiceImpl class without
// defining the actual Bean code in the AppConfig.java class
@Service("speakerService")
public class SpeakerServiceImpl implements SpeakerService {

    private SpeakerRepository repository;

    public SpeakerServiceImpl() {
        System.out.println("SpeakerServiceImpl no arg constructor");
    }

    @Autowired
    public SpeakerServiceImpl(final SpeakerRepository speakerRepository) {
        this.repository = speakerRepository;
    }

    @Override
    public List<Speaker> findAll() {
        return repository.findAll();
    }
}

The repository class —

// This sets up the Bean for HibernateSpeakerRepositoryImpl class
// without writing the Bean definition code in AppConfig.java class
@Repository("speakerRepository")
public class HibernateSpeakerRepositoryImpl implements SpeakerRepository {

    @Override
    public List<Speaker> findAll() {
        List<Speaker> speakers = new ArrayList<>();

        Speaker speaker = new Speaker();
        speaker.setFirstName("Priyanka");
        speaker.setLastName("Masne");

        speakers.add(speaker);

        return speakers;
    }
}

DRY Principal is commonly solved using the Template Method pattern.

public class VacationPolicy {
    public void accrueUSDivisionVacation() {
        // code to calculate vacation based on hours worked to date
        // ...
        // code to ensure vacation meets US minimums
        // ...
        // code to apply vacation to payroll record
        // ...
    }
    public void accrueEUDivisionVacation() {
        // code to calculate vacation based on hours worked to date
        // ...
        // code to ensure vacation meets EU minimums
        // ...
        // code to apply vacation to payroll record
        // ...
    }
}

The code in both the above methods is largely same except for calculation legal minimums. Avoid duplication using Template Method pattern —

public abstract class VacationPolicy {
    public void accrueVacation() {
        calculateBaseVacationHours();
        alterForLegalMinimums();
        applyToPayroll();
    }
    private void calculateBaseVacationHours() { /* ... */ };
    protected abstract void alterForLegalMinimums();
    private void applyToPayroll() { /* ... */ };
}
public class USVacationPolicy extends VacationPolicy {
    @Override
    protected void alterForLegalMinimums() {
        // US specific logic
    }
}
public class EUVacationPolicy extends VacationPolicy {
    @Override
    protected void alterForLegalMinimums() {
        // EU specific logic
    }
}

Reference

These are my learnings from the very famous book by Robert C Martin — Clean Code.