Principles Gathered from Clean Code: A Handbook of Agile Software Craftsmanship

1. The cost of bad code and the essence of clean code

2. Meaningful names

• Reveal the intent of the variable/ function/ class (can I guess what everything does without needing read the logic)

• Avoid disinformation (don't call something accountList unless it really is a list!)

• Avoid non-information (don't call a variable i unless indexing in a loop)

• Make meaningful distinctions - every named "thing" should be something distinct, where the difference between these distinct things should be clear. For example, having car and carInfo is not a helpful distinction (do you really need two variables here if you can't distinctly name them?).

• Are pronounceable - make life easy for yourself!

• Are searchable - make it easy to grep search the entire codebase for the variable you're looking for (unique names trump common ones).

• Are not encoded - don't use prefixes; if your variables are suitably named these shouldn't be required and will, at best, be ignored by readers.

• Avoid “mental mapping” - do not require the reader to translate your name to something else in her head for them to make sense of it.

• Follow a convention - use one name for one concept and stick to it.

• Refer to the problem domain - developers must understand the business domain they are writing software for (rather than mindlessly typing away about things they know nothing about).

• Refer to the solution domain - it will be programmers reading your code, so don't be afraid to use CS terms. I find referring to the problem domain preferable where possible.

• Add meaningful context - add the context that will be required by the reader at the level of abstraction that the reader will be using the variable (the variable state makes sense next to addressLineOne, addressLineTwo but is extremely misleading used out of this context).

• Don't add gratuitous context - if you prefix all variables in the Customer class with customer then every time you type c into your IDE, it will take nine keystrokes before your autocomplete starts helping you out - don't make your IDE work against you!

• Use verbs for functions - verb/ noun pairs are very descriptive (write(name) shows that name is being written - even better would be writeField(name) as we have also described what name is in the function). Also, try to use the same verbs everywhere (don't mix fetch and get)

3. Functions

Small!

Do one thing!

Functions should do one thing. They should do it well. They should do it only.

function login(username, password) {
    if areValidUserCredentials(username, password)
        return initialiseSession(username);

    throw Exception('Invalid user credentials');
}

function areValidUserCredentials (username, password) {
    const user = this.getUserByUsername(username);
    return password === decrypt(user.passwordHash)
}

function login(username, password) {
    const user = this.getUserByUsername(username);
    if (password === decrypt(user.passwordHash)) 
        return initialiseSession(username)
    
    throw Exception('Invalid user credentials');
}

Functions should SLAP! (Single Level of Abstraction Principle)

renderHtml() // high-level 
const pageTitle = getPageSectionTitle() // mid-level
shoppingListToRender.append(shoppingItem) // low-level

function registerUser () {
    const contactDetails = collectUserContactDetails();
    const paymentDetails = collectUserPaymentDetails();
    const user = {...contactDetails, ...paymentDetails};
    createNewUser(user);
}

Fewer arguments

Side effects

function checkPassword (userId, passwordToCheck) {
    const user = getUserById(userId);
    if (user) {
        const encodedPassword = user.passwordHash;
        const userPassword = dcrypt(encodedPassword);
        if (userPassword === passwordToCheck) {
            initializeSession();
            return true;
        };
    }
    return false
}

// (in the body of some outer function)
    ...
    console.log({ allIds }) 
    // output: { allIds: [1, 2, 3] }

    if (!isIdRegistered(id, allIds)) {
        allIds.append(id)
    }

    console.log({ allIds }) 
		// output: { allIds: [4] } - who stole all my ids!?
     
    ...

function isIdRegistered(id, allIds) {
    for (let i=0; i<allIds.length; i++) {
        const registeredId = allIds.pop();
        if (id === registeredId) return true;
    }
    return false;
}

Command-query separation

Extract try/catch blocks

function delete(page) {
    try {
        deletePageAndReferences(page);
    }
    catch (Exception e) {
        logError(e);
    }
}

function deletePageAndReferences(page) {
    deletePage(page);
    registry.deleteReference(page.name);
    configKeys.deleteKey(page.name.makeKey());
}

function delete(page) {
    try {
        deletePage(page);
        registry.deleteReference(page.name);
        configKeys.deleteKey(page.name.makeKey());
    }
    catch (Exception e) {
        console.error(e.message);
        throw e;
    }
}

DRY (don't repeat yourself)

Addendum: DRY (do repeat yourself)

Graph of developer priorities through their career (take a 15-year shortcut; start valuing readability above DRY now)

 

4. Comments

1. Legal comments (e.g. copyright).

2. Revealing intent behind a technical decision which would otherwise be obscure.

3. Clarifying obscure methods of a standard (external) library that you can't refactor (you could also write a unit test for the external library to document it).

4. Warnings to other programmers of the consequences of running some code.

5. TODO comments - these should be immediately documented with a plan to address the tech debt (i.e. could be a link to a ticket where you plan to do the work - don’t create tickets solely to address tech debt, do it alongside a feature)

5. Formatting

6. Objects and data structures

The law of Demeter

final String outputDir = ctxt.getOptions().getScratchDir().getAbsolutePath();

String outFile = outputDir + "/" + className.replace('.', '/') + ".class"; // notice mixed levels of abstraction!
FileOutputStream fout = new FileOutputStream(outFile);
BufferedOutputStream bos = new BufferedOutputStream(fout);

BufferedOutputStream bos = ctxt.createScratchFileStream(classFileName);

Data transfer objects (DTO)

7. Error Handling (by Michael Feathers)

Prefer exceptions to returning error codes

if (deletePage(page) !== CODES.OK){
    console.error('delete failed');
    return CODES.ERROR;
}
if (registry.deleteReference(page.name) !== CODES.OK){
    console.error('deleteReference from registry failed');
    return CODES.ERROR;
}
if (configKeys.deleteKey(page.name.makeKey()) !== CODES.OK){
    console.error('configKey not deleted');
    return CODES.ERROR;
}

console.log('page deleted');
return CODES.OK;

try {
    deletePage(page);
    registry.deleteReference(page.name);
    configKeys.deleteKey(page.name.makeKey());
}
catch (Exception e) {
    console.error(e.message);
    throw e;
}

Use unchecked exceptions

Provide context with exceptions

Define exception classes in terms of the caller's needs

ACMEPort port = new ACMEPort(12);

try {
    port.open();
} catch (DeviceResponseException e) {
    reportPortError(e);
    logger.log("Device response exception", e);
} catch (ATM1212UnlockedException e) {
    reportPortError(e);
    logger.log("Unlock exception", e);
} catch (GMXError e) {
    reportPortError(e);
    logger.log("Device response exception");
} finally {
    ...
}

public class LocalPort {
    private ACMEPort innerPort;

    public LocalPort(int portNumber) {
        innerPort = new ACMEPort(portNumber);
    }

    public void open() {
        try {
            innerPort.open();
        } catch (DeviceResponseException e) {
            throw new PortDeviceFailure(e);
        } catch (ATM1212UnlockedException e) {
            throw new PortDeviceFailure(e);
        } catch (GMXError e) {
            throw new PortDeviceFailure(e);
        }
    }
}

...

LocalPort port = new LocalPort(12);

try {
    port.open();
} catch (PortDeviceFailure e) {
    reportError(e);
    logger.log(e.getMessage(), e);
} finally {
    ...
}

Know when not to use try/catch

try {
    MealExpenses expenses = expenseReportDAO.getMeals(employee.getID());
    m_total += expenses.getTotal();
} catch(MealExpensesNotFound e) {
    m_t += getMealPerDiem();
}

MealExpenses expenses = expenseReportDAO.getMeals(employee.getID());
m_total += expenses.getTotal();

...

public class PerDiemMealExpenses implements MealExpenses {
    public int getTotal() {
        // return the per diem default
    }
}

Avoiding null

1. Don't return null from any of your functions.

2. Don't pass null to any of your functions.

8. Boundaries (by James Grenning)

Using third-party code

Code that is yet to exist

Clean Boundaries

9. Unit testing

The three laws of TDD

1. First Law You may not write production code until you have written a failing unit test.

2. Second Law You may not write more of a unit test than is sufficient to fail, and not compiling is failing.

3. Third Law You may not write more production code than is sufficient to pass the currently failing test.

Keeping clean tests

Clean tests

One assert per test

Single concept per test

FIRST

1. Fast - when they are slow you will be tempted to run them infrequently.

2. Independent - they should be able to run independently and in any order to prevent cascading issues.

3. Repeatable - in any environment (in CI or on your laptop). They should be deterministic, or you will rationalise why they are failing.

4. Self-validating - they should have a boolean output and check their success automatically.

5. Timely - tests should be written in a timely manner (before you write your production code).

Conclusion