Functions

Functions should be small and do one thing.

Small

The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that.

public static List<UserReportDto> getUserReportDtos() {
        List<User> users = CsvUtils.read(new File("user.csv"), User.class);
        List<UserReportDto> userReportDtos = new ArrayList<>();
        if (users != null && !users.isEmpty()) {
            for (User user : users) {
                UserReportDto reportDto = UserReportDto.builder()
                        .withName(user.getName())
                        .withEmail(user.getEmail())
                        .build;
                userReportDtos.add(reportDto);
            }
        }
        return userReportDtos;
}
// First level of abstraction
public static List<UserReportDto> getUserReportDtos() {
    List<User> users = readUsersFromCsvFile("user.csv");
    return mapToUserReportDtos(users);
}

// Second level of abstraction
private List<User> readUsersFromCsvFile(String csvFileName) {
    return CsvUtils.read(new File(csvFileName), User.class);
}

// Second level of abstraction
private List<UserReportDto> mapToUserReportDtos(List<User> users) {
    List<UserReportDto> userReportDtos = new ArrayList<>();
    if (!isValid(users)) {
        return userReportDtos;
    }
    for (User user : users) {
        UserReportDto reportDto = mapToUserReportDto(user);
        userReportDtos.add(reportDto);
    }
    return userReportDtos;
}

// Third level of abstraction
private boolean isValid(List<User> users) {
    return users != null && !users.isEmpty();
}

// Third level of abstraction
private UserReportDto mapToUserReportDto(User user) {
    return UserReportDto.builder()
                .withName(user.getName())
                .withEmail(user.getEmail())
                .build();
}

Refer to Write Short Units of Code

Do One Thing

One way to know that a function is doing more than “one thing” is if you can extract another function from it with a name that is not merely a restatement of its implementation.

See examples in the "Small" section.

One Level of Abstraction per Function

In order to make sure our functions are doing “one thing,” we need to make sure that the statements within our function are all at the same level of abstraction.

Abstraction is a fundamental concept in OOPS. It talks about hiding the “how” part and only exposing “what” to the outer world.

According to SLAP (Single Level of Abstraction Principle), source code inside each method should refer to concepts and mechanisms relevant to just one level of “operational complexity” of your application.

Apply refactoring technique "Extract Method".

See examples in the "Small" section.

Reading Code from Top to Bottom: The Stepdown Rule

We want every function to be followed by those at the next level of abstraction so that we can read code from top to bottom.

See examples in the "Small" section.

Avoid nested structures

public boolean isUserValid(UserDto userDto) {
    if (userDto != null) {
        if (userDto.getPassword() != null) {
            if (userDto.getEmail() != null) {
                return !userService.isEmailExists(userDto.getEmail());
            }
        }
    }
    return false;
}

public boolean isUserValid(UserDto userDto) {
    if (userDto == null) {
        return false;
    }
    if (userDto.getPassword() == null) {
        return false;
    }
    if (userDto.getEmail() == null) {
        return false;
    }
    return !userService.isEmailExists(userDto.getEmail());
}

Avoid negative conditionals

Negatives are just a bit harder to understand than positives. So, when possible, conditionals should be expressed as positives.

if(!isNotEmpty(users)) {}
if(isEmpty(users)) {}

Switch Statements

It’s hard to make a small switch statement. By their nature, switch statements always do N things. Unfortunately, we can’t always avoid switch statements, but we can make sure that each switch statement is buried in a low-level class and is never repeated. We do this, of course, with polymorphism.

There are several problems with the switch statement.

  • First, it’s large, and when new enum types are added, it will grow.

  • Second, it very clearly does more than one thing.

  • Third, it violates the Single Responsibility Principle (SRP) because there is more than one reason for it to change.

  • Fourth, it violates the Open Closed Principle (OCP) because it must change whenever new types are added.

enum VehicleType {
    BICYCLE,
    CAR,
    AIRPLANE
}

class Vehicle {
    String name;
    VehicleType type;
    void ride() {}
    void fly() {}
}

void perform(Vehicle vehicle) {
    switch (vehicleType) {
        case BICYCLE:
            vehicle.ride();
            break;
        case CAR:
            vehicle.drive();
            break;
        case AIRPLANE:
            vehicle.fly();
            break;
        default:
            break;
    }
}

interface VehicleAction {
    void perform();
}

abstract class Vehicle implements VehicleAction {
    String name;
}

class Bicycle extends Vehicle {

    void ride() {}

    @Override
    public void perform() {
        ride();
    }
}

class Car extends Vehicle {
    void drive() {}

    @Override
    public void perform()
        drive();
    }
}

// App
void perform(VehicleAction vehicleAction) {
    vehicleAction.perform();
}

Use Descriptive Names

Don’t be afraid to make a name long. A long descriptive name is better than a short enigmatic name. A long descriptive name is better than a long descriptive comment.

public void createPrd(); // Create a product
public void createProduct();

Be consistent in your names. Use the same phrases, nouns, and verbs in the function names you choose for your modules.

beforeEach();

afterEach();

beforeAll();

afterAll();

Prefer fewer arguments

The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.

void createUser(String username, String password, Gender gender, int roleId);

Apply refactoring technique "Replace Method Parameters with Method Object": When a function seems to need more than two or three arguments, it is likely that some of those arguments ought to be wrapped into a class of their own.

void createUser(UserDto userDto) {
}

class UserDto {
    private String username;
    private String password;
    private Gender gender;
    private int roleId;
}

Functions that take variable arguments can be monads, dyads, or even triads. But it would be a mistake to give them more arguments than that.

Arguments are even harder from a testing point of view. Imagine the difficulty of writing all the test cases to ensure that all the various combinations of arguments work properly.

Sometimes we want to pass a variable number of arguments into a function. Consider to use Argument Lists

public String format(String format, Object… args);

String.format(”%s worked %.2f hours.”, name, hours);

Read more: Keep Unit Interfaces Small

Avoid flag arguments

Function with flag argument does more than one thing. It does one thing if the flag is true and another if the flag is false!

public execute(boolean isTestSuite) {
    if (isSuite) {
        // do something for suite
    } else {
        // do something for single test
    }
}

public void executeForTestSuite() {
    // do something for suite
}

public void executeForSingleTest() {
    // do something for single test
}

Split the method into several independent methods that can be called from the client without the flag.

Have No Side Effects

Side effects are lies. Your function promises to do one thing, but it also does other hidden things.

The side effect is the call to update(). The get user function, by its name, says that it gets the user by email. The name does not imply that it updates the last access of the user.

public User getUserByEmail(String email) {
        User user = userRepository.getUserByEmail(email);
        user.setLastAccess(now());
        userRepository.update(user);
        return user;
    }

Output Arguments

Anything that gets modified or mutated but is not returned by the function. An array/object passed to a function, which changes the body of the array or changes the data of the object but does not return it, then it is the output argument.

public User createUser(UserDto userDto) {
    userDto.setRole("ADMIN");
    return userRepository.createUser(userDto);
}

class UserDto {
    private String email;
    private String password;
    private String role;
}

Output arguments should be avoided. If your function must change the state of something, have it change the state of its owning object.

public void appendFooter(StringBuffer report);

appendFooter(report);

report.appendFooter();

Separate command from query

Functions should either do something or answer something, but not both.

public boolean set(String attribute, String value);

if (set("username", "gpcoder")) {}

if (attributeExists("username")) {

setAttribute("username", "gpcoder");

}

Prefer exceptions to returning error codes

Returning error codes from command functions lead to deeply nested structures and maybe a violation of command query separation.

public void sendShutDown() {
    DeviceHandle handle = getHandle(DEV1);

    // Check the state of the device
    if (handle != DeviceHandle.INVALID) {

        // Save the device status to the record field
        retrieveDeviceRecord(handle);

        // If not suspended, shut down
        if (record.getStatus() != DEVICE_SUSPENDED) {
            pauseDevice(handle);
            clearDeviceWorkQueue(handle);
            closeDevice(handle);
        } else {
            logger.log("Device suspended.  Unable to shut down");
        }
    } else {
        logger.log("Invalid handle for: " + DEV1.toString());
    }
}
public void sendShutDown() {
    try {
        tryToShutDown();
    } catch (DeviceShutDownError e) {
        logger.log(e);
    }
}

private void tryToShutDown() throws DeviceShutDownError {
    DeviceHandle handle = getHandle(DEV1);
    
    DeviceRecord record = retrieveDeviceRecord(handle);

    pauseDevice(handle);
    clearDeviceWorkQueue(handle);
    closeDevice(handle);
}

private DeviceHandle getHandle(DeviceID deviceID) {
    throw new DeviceShutDownError("Invalid handle for: " + deviceID);
}

Another exmaple:

public void performLogin(UserDto userDto) {
    ResponseCode status = login(userDto);
    switch (status) {
        case BAD_REQUEST:
            log.error("Bad request");
            break;
        case USER_NOT_FOUND:
            log.error("Invalid username or password");
            break;
        case WRONG_PASSWORD:
            log.error("Invalid username or password");
            break;
        case SUCCESS:
            log.error("Login success");
            redirect();
            break;
        default:
            log.error("Unknown response");
            break;
    }
}

public ResponseCode login(UserDto userDto) {
    if (userDto.getEmail() == null || userDto.getPassword() == null) {
        return ResponseCode.BAD_REQUEST;
    }
    User user = userRepository.get(userDto);
    if (user == null) {
        return ResponseCode.USER_NOT_FOUND;
    }
    if (notMatchPassword(user.getPassword, userDto.getPassword())) {
        return ResponseCode.WRONG_PASSWORD;
    }
    setUserSession(user);
    return SUCCESS;
}

public void performLogin(UserDto userDto) {
    try {
        login(userDto);
    } catch(Exception ex) {
        log.error(ex.getMessage(), ex);
    }
    redirect();
}

public void login(UserDto userDto) {
    if (userDto.getEmail() == null || userDto.getPassword() == null) {
        throw new BadRequestException("Bad request");
    }
    User user = userRepository.get(userDto);
    if (user == null) {
        throw new UserNotFoundException("Invalid username or password");
    }
    if (notMatchPassword(user.getPassword, userDto.getPassword())) {
        throw new UserNotFoundException("Invalid username or password");
    }
    setUserSession(user);
}

When you use exceptions rather than error codes, then new exceptions are derivatives of the exception class. They can be added without forcing any recompilation or redeployment.

It is better to extract the bodies of the try and catch blocks out into functions of their own.

Put all code inside a try block

If the keyword try exists in a function, it should be the very first word in the function and there should be nothing after the catch/finally blocks.

public void performLogin(UserDto userDto) {
    try {
        login(userDto);
    } catch(Exception ex) {
        log.error(ex.getMessage(), ex);
        throw new BadCredentialExceptoin();
    }
    redirect(); // Should move to try block
}

public void performLogin(UserDto userDto) {
    try {
        login(userDto);
        redirect();
    } catch(Exception ex) {
        log.error(ex.getMessage(), ex);
        throw new BadCredentialExceptoin();
    }
}

The DRY principle

Refer to DRY Principle

Structured Programming

Every block within a function should have one entry and one exit. Following these rules means that there should only be one return statement in a function, no break or continue statements in a loop, and never, ever, any goto statements.

If you keep your functions small, then the occasional multiple return, break, or continue the statement does no harm and can sometimes even be more expressive than the single-entry, single-exit rule. On the other hand, goto only makes sense in large functions, so it should be avoided.

Write, test, and refine

When I write functions, they come out long and complicated. But I also have unit tests that cover every one of those clumsy lines of code. So then I massage and refine that code, splitting out functions, changing names, eliminating duplication.

— Robert C. Martin

PreviousMeaningful NamesNextClasses

Last updated