2. Functions - tuansondinh/clean_code GitHub Wiki

2.1 Small Functions

GOOD: - Maximum 100 lines

BETTER:- should fit in one screen

BEST:- not more than 20 lines

Blocks in if, else, while etc. should be One-Liner (probably function call)

GOOD:

if(...) doSomething();
else doSomethingElse();

2.2 Function should do One Thing -> Single Responsiblity Princip (SRP)

2.3 One Level of Abstraction per Function

Topdown-Rule: Code should be readable like a story from Top to Bottom.

Stepdown-Rule: Behind every function, there are other functions in the next lower abstraction level.

BAD:

public createCars(){
   createOpel();
   Audi audi = new Audi();
}

GOOD:

public createCars(){
   createOpel();
   createAudi();
}

2.4 switch statements

BAD:

public Money calculatePay(Employee e) throws InvalidEmployeeType {
   switch (e.type) {
      case COMMISSIONED:
         return calculateCommissionedPay(e);
      case HOURLY:
         return calculateHourlyPay(e);
      case SALARIED:
         return calculateSalariedPay(e);
   default:
         throw new InvalidEmployeeType(e.type);
   } 
}

Reasons:

1. Big Function. Gets even bigger, if new types are added
2. Does more than one thing.
3. Violates against SRP. (More than one reason for a change.)
4. Violates against Open Closed Princip (OCP). (Must be changed, if new types are added)
5. There could be unlimited functions with same structure like:

isPayday(Employee e, Date date)

SOLUTION: put switch statements inside Abstract Factory

public abstract class Employee {
    public abstract boolean isPayday();
    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 COMMISSIONED:
                return new CommissionedEmployee(r);
            case HOURLY:
                return new HourlyEmployee(r);
            case SALARIED:
                return new SalariedEmploye(r);
            default:
                throw new InvalidEmployeeType(r.type);
        }
    }
}

2.5 Use Descriptive Names

• name should describe the action of the function
• long names are better than long comments or short, but not very meaningful names
• be consistent in your names (use same phrases, nouns, verbs

2.6 Function arguments

• the less arguments, the better (easier to understand and test)
• BEST: 0 arguments, MAXIMUM: 3 arguments
• use Input Arguments instead of Output Arguments

2.6.1 Monadic Function (1 argument)

case 1 - ask a question about the argument

boolean fileExists("MyFile")

case 2 - transform the argument and return it

InputStream fileOpen("MyFile")

case 3 - Function call is an event; use the argument to alter the state of the system; no output argument

void passwordAttemptsFailedNtimes(int attempts)

if possible, use only those 3 cases, when using Monadic Functions

2.6.2 Avoid Flag/Boolean Arguments

• hard to understand,
• function does more than one thing (2 cases -> true and false)

2.6.3 Dyadic functions (2 arguments)

• in some cases it's the right solution, like

Point p = new Point(0,0)

• but other than that, it should be avoided

2.6.4 Triadic Functions

• AVOID! But there are some exceptions like:

assertEquals(1.0, amount, .001)

2.6.5 Argument-Objects

If possible, arguments should be wrapped into a class, like:

Circle makeCircle(double x, double y, double radius); //before
Circle makeCircle(Point center, double radius); //after and better

2.6.6 Argument-Lists

2.6.7 verbs and keywords

monad functions:use nice verb/noun pair for function and argument:

writeField(name);

here we know, that 'name' is a field, that is being written

other functions: encode the argument names into the function names:

BAD:

assertEquals(expected, actual); 

GOOD:

assertExpectedEqualsActual(expected, actual); 

This way we don't have to remember the order.

2.7 Avoid side effects

BAD: "Your function promises to do one thing, but it also does other hidden things" (S.44) Example:

public boolean checkPassword(String userName, String password){
 ...// logic for checking password
 Session.initialize();
}

The function name does not imply that it initializes a session!

GOOD:

checkPasswordAndInitializeSession(userName, password);

BETTER: Function should only do ONE thing

Output Arguments

Generally you should not use Output Arguments! BAD:

// this function appends a footer to the argument (report) 
public void appendFooter(StringBuffer report){...}

GOOD: it's the task of this to work as an 'output argument'

report.appendFooter();

2.8 separate command and query

BAD: This function below sets an attribute with a value (command) and returns true (query), if it was successful and false, if attribute doesn't exist.

public boolean set(String attribute, String value);

This will lead to:

if(set("username","unclebob"))...

GOOD:

if(attributeExists("username")){ // query
 setAttribute("username", "unclebob"); //instruction
}

2.9 Exception are better than Error Codes

command-functions returning Error Codes are violating againts command-query-separation and will lead to deeply nested if statements

BAD:

if(deletePage(page) == E_OK){
  if(registry.deleteReference(page.name) == E_OK){
    ...

Use try/catch to separate error processing code from 'normal processing' code. Also the blocks should be oneliner(function call).

GOOD:

// task of this function is error processing **only**, therefore it should only contain try and catch/finally block
public void delete(Page page){
  try {
    deletePageAndAllReferences(page);
  }
  catch (Exception e){
    logError(e);
  }
}

The Error.java Dependency Magnet

Avoid using Error Codes, because they need an enum or class where they are defined. Problem: other classes are dependent of this enum. if enum is changed -> recompiling, redeployment

public enum Error {
 OK,
 INVALID,
 NO_SUCH,
}

Use Exceptions instead, because new Exceptions are subclasses of Exception-> no recompiling, redeployment -> Open-Closed-Principle(OCP)

2.10 Don't Repeat Yourself

This one is obvious. Duplicate code will make your code unnecessarily bigger and the effort is great, when you have to change something. "Duplication may be the root of all evil in software." S.48

2.11 Structured Programming

For Bigger Functions, go for following rule:

"Dijkstra said that every function, and every block within a function, should have one entry and one exit." S.48

-> only 1 return statement in a function

-> no break or continue statements in a loop

-> no goto

2.12 But how to write those clean functions?

Just start to write your function. It doesn't have to be clean right from the start.

Refactor and make your code clean afterwards!

2.13 Summary

"Master programmers think of systems as stories to be told rather than programs to be written." (S.50)