I am sure everybody who is reading this post, reads a lot of code on a daily basis. Have you ever encountered a class or script that houses big functions, that does too many things? Or deeply nested control structures? Or confusing flows that you do not want to visit again? These are some problems that we encounter from time to time while reading code. The code works but it is not easily understandable or even maintainable. Clean code is easy to read and make sense of and thus makes it easy to maintain.
Like an author of a book, one should write a good and readable story when coding. I know it becomes difficult to stay clean when there are deadlines, or when there is a vacation coming that you have been waiting to go to since last year and you need to wrap up a functionality before catching that flight. So you end up writing a quick code instead of a clean code. It does the job but it does not follow the best practices that can save time and increase productivity in the future.
So how do we always write clean code?
We cannot, at least not in the first go. Writing clean code is an iterative process. Think of it as your first draft when writing a story, keep it clean as much as possible. Keep an open mind and look out for improvements when you revisit that function while fixing a bug or extending that class and then refactor.
Let’s dive into writing clean code
We are going to look at a few best practices that will help us in writing clean code. These guidelines are not something that I invented, but these have been curated from various other helpful articles and few are based on my experience. I have used Groovy and Javascript (ES6) for code examples. These best practices are relevant to all programming languages.
Naming Convention
We should consciously make an effort to follow the proper naming conventions for variables, properties, functions/methods, and classes. When naming these, be descriptive. The name should convey one simple purpose. It should describe what is stored in the variable, property or what function does it perform in the given function or class.
Rules to follow for a clean code here
- For Variables/Properties
Should imply what kind of data is stored, typically a noun as a name. eg: employee, organization, customer, etc., or short phrases containing adjectives. eg: isLoggedIn, isValid, didAuthenticate etc.
Also, be as specific as you can be eg: Prefer Orange/Tomato instead of Eatables
- For Functions/Methods
Functions/methods perform some task or operation and it should be typically named as a verb. eg: login(), connectDatabase(), processRequest() etc or short phrases containing adjectives. eg: isAuthorized(), isValidSession(), isEmpty() etc.
- For Classes
Used to create objects, initialize properties, and implement the functionality. In the end, they perform tasks and operations. as a best practice name them as nouns. eg: User, AuthenticationController, PaymentHelper etc
- Avoid Generic Names
Try to avoid generic names wherever possible like process(), handler(), a data, read() are very generic. Instead, use processRequest(), eventHandler(), userData, etc.
- Be consistent
The key is consistency. Follow the same rules everywhere. Do not use getUsername if you used fetchProductName earlier in the other part of your code.
Formatting
Proper code formatting helps with better readability and understanding. Try to keep statements within the screen limit (I mean in a 14 or 15 in screen), if it exceeds try to split it into multiple lines. It is all about grouping related concepts together and adding space/line between different concepts.
boolean checkAuthorization(def userName, PostgresService postgresService) {
if (postgresService.equals("null")) {
return false
}
def userPrivileges = DatabaseConnection.newInstance(postgresService).getUserPrivileges(userName)
final def allowedUserPrivileges = AppConfigUtil.instance.getAllowedUserPrivileges()
final boolean isValidPrivilege = comparePrivileges(userPrivileges, allowedUserPrivileges)
if (isValidPrivilege) {
return true
} else {
throw new UnauthorizedException("User is not authorized")
return false
}
}
void login(def userName, def password) {
try {
if (userName.equals("null") || password.equals("null")) {
throw new InvalidCredentialException("User or password is invalid")
}
boolean isAuthorized = checkAuthorization(userName, postgresService)
if (isAuthorized) {
saveToDatabase(postgresService)
} else {
println "${userName} is not authorized"
}
} catch (UnauthorizedException unauthorizedException) {
println unauthorizedException
} catch (InvalidCredentialException invalidCredentialException) {
println invalidCredentialException
}
}
The above code looks fine. It follows proper naming convention, it is not very long, but the readability suffers here. Instead, compare this with the below code snippet. It’s the exact same code with a few added blank lines to improve the readability.
boolean checkAuthorization(def userName, PostgresService postgresService) {
if (postgresService.equals("null")) {
return false
}
def userPrivileges = DatabaseConnection.newInstance(postgresService).getUserPrivileges(userName)
final def allowedUserPrivileges = AppConfigUtil.instance.getAllowedUserPrivileges()
final boolean isValidPrivilege = comparePrivileges(userPrivileges, allowedUserPrivileges)
if (isValidPrivilege) {
return true
} else {
throw new UnauthorizedException("User is not authorized")
return false
}
}
void login(def userName, def password) {
try {
if (userName.equals("null") || password.equals("null")) {
throw new InvalidCredentialException("User or password is invalid")
}
boolean isAuthorized = checkAuthorization(userName, postgresService)
if (isAuthorized) {
saveToDatabase(postgresService)
} else {
println "${userName} is not authorized"
}
} catch (UnauthorizedException unauthorizedException) {
println unauthorizedException
} catch (InvalidCredentialException invalidCredentialException) {
println invalidCredentialException
}
}
Rule of thumb on where to add blank lines
- Group statements together based on connection
- Separate statements that are not related closely.
Ordering of functions or methods: Follow a top-down approach here. If function A is called by function B, then function A should be ordered just below function B.
Functions or Methods
A function should perform only one job, be concise, and have fewer parameters. Here are few things that matter in clean function.
- Both method calling and method implementation should be easily readable and maintainable.
- The number and order of function parameter matters.
- Minimize the number of parameters.
- The length of the function body also matters.
Check out below signals on when to stop extracting your functions.
- You are just renaming an operation.
- Suddenly you realize you have to scroll more for a given implementation.
- You cannot figure out a reasonable name for the new function.
Control Structures
Control structures like, if statements, for loops, while loops and switch statements are always part of your code and they help in the code flow. Follow the below checks.
- Prefer positive checks
Check below conditions
if (isValid(emailAddress)) {
return true
}
is (!isEmpty(phonenumber)) {
// throws Exception
}
The only exception here is when there are more options and a combination of checks is involved. In this case, it is better to use a negative check.
Here the 1st snippet is better than the second snippet with a negate ! sign. It makes us think a little more than the first one.
- Avoid deep nesting
Absolutely avoid deeply nested control statements. Use guards, fail fast, split logic into multiple functions, polymorphism & factory functions, and embrace errors.
Check the below code snippet
void sendNotification(def user, def message) {
def notificationStatusMsg = "Error"
if (user) {
if (message) {
if (user.isNotificationEnabled()) {
user.sendNotification(message)
if (send()) {
notificationStatusMsg.append("Notification sent successfully to ${user.name}")
} else {
notificationStatusMsg.append("Error while sending notification to ${user.name}")
}
}
}
}
println notificationStatusMsg
}
and here is the same improved code snippet that shows guards and fail safe by adding if condition in the beginning.
void sendNotification(def user, def message) {
def notificationStatusMsg = "Error"
if (!user || !message || !user.isNotificationEnabled()) {
println notificationStatusMsg
return;
}
user.sendNotification(message)
if (send()) {
notificationStatusMsg.append("Notification sent successfully to ${user.name}")
} else {
notificationStatusMsg.append("Error while sending notification to ${user.name}")
}
println notificationStatusMsg
}
- Polymorphism and factory function example
Avoid multiple if statements and deep nesting using polymorphism and factory functions. Have a look at below code snippet.
const checkRequest = (requestJson) => {
let processors = {
processLogin: null,
processLoggedInRequests: null
}
if (requestJson.type === "sso") {
processors.processLogin = processSso(requestJson);
processors.processLoggedInRequests = processSsoGeneral(requestJson);
}
if (requestJson.type === "standalone") {
processors.processLogin = processStandalone(requestJson);
processors.processLoggedInRequests = processStandaloneGeneral(requestJson);
}
}
const processSso = request => {
console.log(`Processing SSO request for ${request.username} user`)
}
const processStandalone = request => {
console.log(`Processing Standalone request for ${request.username} user`)
}
const processSsoGeneral = request => {
console.log(`Processing SSO general request for ${request.username} user`)
}
const processStandaloneGeneral = request => {
console.log(`Processing Standalone general request for ${request.username} user`)
}
const requestJson = {"op":"login", "type":"sso", "username":"aabingunz", "session": 123456}
const requestProcess = checkRequest(requestJson);
if (isLogin(requestJson)) {
requestProcess.processLogin();
} else {
requestProcess.processLoggedInRequests();
}
const isLogin = request => {
if (request.op === "login") {
return true;
} else {
return false;
}
}
Now let’s refactor this code using polymorphism and factory function.
const checkRequest = (requestJson) => {
let processors = {
processLogin: null,
processLoggedInRequests: null
};
if (requestJson.type === "sso") {
processors.processLogin = processSso;
processors.processLoggedInRequests = processSsoGeneral;
}
if (requestJson.type === "standalone") {
processors.processLogin = processStandalone;
processors.processLoggedInRequests = processStandaloneGeneral;
}
return processors;
}
const processSso = request => {
console.log(`Processing SSO request for ${request.username} user`);
}
const processStandalone = request => {
console.log(`Processing Standalone request for ${request.username} user`);
}
const processSsoGeneral = request => {
console.log(`Processing SSO general request for ${request.username} user`);
}
const processStandaloneGeneral = request => {
console.log(`Processing Standalone general request for ${request.username} user`);
}
const requestJson = {"op":"login", "type":"sso", "username":"aabingunz", "session": 123456};
const processors = checkRequest(requestJson);
if (isLogin(requestJson)) {
processors.processLogin(requestJson);
} else {
processors.processLoggedInRequests(requestJson);
}
const isLogin = request => {
if (request.op === "login") {
return true;
} else {
return false;
}
}
Here the processLogin and processLoggedInRequests is moved under a factory function that produces relevant objects and runs only once.
The checkRequest() function returns a polymorphic object with 2 properties that store functions (These functions are not called yet.
Note: function call () is missing after processSso & processSsoGeneral).
The returned object is polymorphic which means the calling logic never changes but the logic that gets executed can be different. This problem can even be solved using classes and inheritance.
- Use throw and catch exceptions example
Almost all programming language supports to try and catch block, use it to effectively get rid of multiple if checks.
Once an error occurs it will bubble up canceling every execution through the entire call stack until it reaches the exception handler. This removes the need for extra if checks and return statements at multiple places. To give you an idea, here is an example code snippet.
const loginHandler = request => {
try {
checkUser(request.emailid);
} catch(error) {
console.error(error.message);
}
}
const checkUser = emailid => {
validateUser(emailid);
// other important statements
}
const validateUser = emailid => {
const regex = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
if (!regex.test(String(emailid).toLowerCase())) {
throw new Error(`Invalid email id ${email}`);
}
const isUserPresent = checkUserPresence();
if (!isUserPresent) {
throw new Error(`${emailid} is not found!`);
}
}
Comments
Comments in code are good practice, but sometimes it is the opposite. Here we will look at some good and bad examples of comments.
Some examples of bad comments.
-
Misleading comments: You have a
loginfunction but the comment says// create a new user
- Commented out code: We have source control managers, your code is safe. Ditch the commented code, it just bloats your file.
- Dividers and Markers: Avoid these, since you are going to have to write code in a clean way. It will just interrupt the reading flow.
- Redundant information: Avoid unnecessary information in your comment eg:
function connectDatabase() {
// Connect to database
}
Here the comment is redundant since we are already following best practices to name our function and this comment does not give any other useful information.
Some examples of good comments.
Legal information: Some projects require you to add legal comments at the beginning of the file. Go ahead and add it. eg: // (c) Aabingunz
Important information: If the function or variable is used to perform a certain job that has some rules, then it is a good idea to add this information in the comments. eg: // password strength minimum 8 characters long, a combination of 1 upper case, lower case and special character atleast.
Warnings: In some rare cases a warning can save time for other developers. eg: Given snippet warning makes sense because in doFilter implementation is only meant for a staging and production environment.
// Warning: This should only work in the staging and production environment, try to skip this by setting a development environment variable
@Override
Publisher<MutableHttpResponse<?>> doFilter(HttpRequest < ? > request, FilterChain chain) {
...
}
To-do notes: Do not overdo with comments in To-do or description.
Glossary
Noun: A name given to a specific function for a given object. eg: living things, actions, qualities, etc
Adjective: An attribute given to a noun. eg: sweet candy is authenticated, did login, etc.
Verb: A word that conveys action.
That’s all in this post, we will dive deeper into writing clean code in the world of Objects, Classes, Structures etc in the next post.
0 Comments