Clean Code: Functions

Vignesh Sathiyamurthy
4 min readAug 27, 2022

It has always been difficult for developers to write clean functions. Some of my observations that could help you write clean and reusable functions into your applications.

Function names need to be descriptive and meaningful, don’t be afraid to do so as long as they are descriptive.

public User createUser() {}public User createOrUpdateUser() {}public List<Discussion> fetchDiscussionsFromCommunities() {}public void deletUserIfSubscriptionExpired() {}

Function names should be verbs, name should clearly state the action they perform.

public User fetchUserByEmail(String email) {}public boolean isRecommendedArticle(Article article) {}

Pick one word per concept is abstract and stick with it. It is recommended that we have uniform names in the code.

# bad practice
public List<Message> fetchMessages() {}
public List<Notification> getNotifications() {}public List<Subscriber> retrieveSubscribers() {}# good practice
public List<Message> fetchMessages() {}
public List<Notification> fetchNotifications() {}public List<Subscriber> fetchSubscribers() {}

Pass few arguments as possible, ideally none.

# bad practice 
public Movie createMovie(String name, String genre,
dobule duration, ...) {}
# good practice
class CreateMovieRequest {
private String name;
private String genre;
double duration;
...
}
public Movie createMovie(CreateMovieRequest createRequest) {}public void deleteActor(String id) {}class FilterMovieRequest {
private String name;
private String actor;
private String genre;
private String allSearchQuery;
}

public List<Movie> fetchMovies(FilterMovieRequest filterRequest) {}

Avoid passing boolean as an argument why do we need to use boolean as an argument? Should we do more than one thing in one function? Then write a separate function to it.

Add pre-conditions to the required arguments to avoid the null pointer exception. Inject com.google.guava dependeny in your pom.xml or build.gradle to add pre-conditions.

public void deleteUser(String id) { 
Preconditions.checkArgument(StringUtils.isNotEmpty(id), "user id cannot be null");
}
class FilterUserRequest {
private String groupId;
private String allSearchQuery;
}
public void fetchUsersFromGroup(FilterUserRequest filterRequest) {
Preconditions.checkArgument(ObjectUtils.isNotEmpty(filterRequest), "filter user request cannot be null");
Preconditions.checkArgument(StringUtils.isNotEmpty(filterRequest.getGroupId), "group id cannot be null");
}

Functions should short and do only one thing. If your functions is doing more than one thing, it is perfect moment to extract to another function.

Avoid side effects, if you do that “one thing” and also change the state of the system, you are doing more things.

# bad practice
public User createUserIfNotFound(CreateUserRequest createRequest) {
User existingUser = userRepository.fetchUserByEmail(createRequest.getEmail();
if (existingUser != null) {
return existingUser;
}
User user = new User();
user.setName(createRequest.getName());
user.setEmail(createRequest.getEmail());
user.setAddress(createRequest.getAddress());
return userRepository.save(user);
}
# good practice - created separate function to create user if not existspublic User createUserIfNotFound(CreateUserRequest createRequest) {
User existingUser = userRepository.fetchUserByEmail(createRequest.getEmail();
if (existingUser != null) {
return existingUser;
}
return createUser(createRequest);
}
public User createUser(CreateUserRequest createRequest) {
User user = new User();
user.setName(createRequest.getName());
user.setEmail(createRequest.getEmail());
user.setAddress(createRequest.getAddress());
return userRepository.save(user);
}

Smaller functions easy to understand and maintain.

public UserStats fetchUserStats(String id) {
User user = fetchUserById(id);
Date lastLoginDate = fetchUserLastLoginDate(id);
Integer totalMessages = fetchUserTotalMessages(id);
Integer userMentions = fetchUserMentions(id);
UserStats stats = new UserStats();
stats.setUser(user);
stats.setLastLoginDate(lastLoginDate);
stats.setTotalMessages(totalMessages);
stats.setUserMentions(userMentions);

return stats;
}
private User fetchUserById(String id) {
...
}
private Date fetchUserLastLoginDate(String id) {
...
}
private Integer fetchUserTotalMessages(String id) {
...
}
private Integer fetchUserMentions(String id) {
...
}

Functions should not exceed 20 lines and generally less than 10 lines. Functions are longer than 20 lines considered as messy functions.

Don’t repeat by yourself - Avoid duplicate code, write a reusable functions.

# bad practice
public User fetchUser(String email) {
User user = userRepository.fetchUserByEmail(email);
if (!user.isEnabled()) {
throw new InvalidUserExeption();
}

return user;
}
public User updateUser(String email, UpdateUserRequest updateRequest) {
...
User existing = userRepository.fetchUserByEmail(email);
if (!existing.isEnabled()) {
throw new InvalidUserExeption();
}

existing.setName(updateRequest.getName());
existing.setMobile(updateRequest.getMobile());
...
return userRepository.save(existing);
}
public void validateUser(String email) {
User user = userRepository.fetchUserByEmail(email);
if (!user.isEnabled()) {
throw new InvalidUserExeption();
}
}
# good practice
private User fetchValidUser(String email) throws InvalidUserExeption {
User user = userRepository.fetchUserByEmail(email);
if (!user.isEnabled()) {
throw new InvalidUserExeption();
}

return user;
}
public User fetchUser(String email) throws InvalidUserExeption {
return fetchValidUser(email);
}
public User updateUser(String email, UpdateUserRequest updateRequest) {
User existing = fetchValidUser(email);
existing.setName(updateRequest.getName());
existing.setMobile(updateRequest.getMobile());
...
return userRepository.save(existing);
}
public void validateUser(String email) throws InvalidUserExeption {
fetchValidUser(email);
}

Encapsulate the conditional, instead of writing several lines of code inside the condition block, put it to separate function that makes your code more readable. This means that if statements, for and while loops should be one line long, obviously this line should be calling another function.

int NON_SUBSCRIBER_READ_LIMIT = 5;# bad practice
public boolean allowUserToReadArticle(User user) {
if (user.isSubscribedMember()) {
return true;
} else {
int noOfReadArticles = fetchUserReadArticlesCount(user);
return noOfReadArticles < NON_SUBSCRIBER_READ_LIMIT;
}
}
# good practice
public boolean allowUserToReadArticle(User user) {
if (user.isSubscribedMember()) {
return true;
}

return isNonSubsciberReadRemaining(user);
}
public boolean isNonSubsciberReadRemaining(User user) {
int noOfReadArticles = fetchUserReadArticlesCount(user);
return noOfReadArticles < NON_SUBSCRIBER_READ_LIMIT;
}

Do not leave your commented code, others afraid to delete your commented code because they do not know if it is there for a reason and it will stay forever.

Style guides available for many programming languages, it will help you write clean code and remind yourself that it changes from one programming language to the next.

Hope my observations can help! See you in the near future with my next article Clean Code: Classes.

Thanks 😊

--

--