DEV Community

Cover image for Coding Best Practices, Chapter One: Functions.
Levi Velázquez
Levi Velázquez

Posted on • Edited on

Coding Best Practices, Chapter One: Functions.

My purpose IS NOT to say what is the best way to write code, some programmers have their ways and they are just fine. I just want to share with others what I consider code best practices and how to improve several aspects as readiness, structure, components, meaning, debugging, refactoring, etc.

Chapter 1: Functions

Do One Thing.

I wanted to start from the most fundamental rule and according to me, the best practice: The One Thing Rule. It will make our life easier, try to create your functions to Do One Thing.

It could be very hard to write functions that do just one thing, but it should be our ultimate goal in our daily process. Why? Let's look a this

def create_user(email, password):
    try:
        user = User.objects.get(email=email)
    catch UserDoesNotExist:
        user = User.objects.create(
            email=email,
            password=password
        )
        auth_valid = authenticator(
            user=user,
            password=password
        )
        if auth_valid:
            user.last_login = datetime.now()
            user.save()
            msg_content = '<h2>Welcome {email}</h2>'.format(
                email=email
            ) 
            message = MIMEText(
                msg_content,
                'html'
            )
            message.update({
                'From': 'Sender Name <sender@server>',
                'To': email,
                'Cc': 'Receiver2 Name <receiver2@server>',
                'Subject': 'Account created'
            })
            msg_full = message.as_string()
            server = smtplib.SMTP('smtp.gmail.com:587')
            server.starttls()
            server.login(
                'sender@server.com',
                'senderpassword'
            )
            server.sendmail(
                'sender@server.com',
                [email],
                msg_full
            )
            server.quit()
            return user
Enter fullscreen mode Exit fullscreen mode

What this function does ?

  • If the user doesn't exist, it creates a new user.
  • It creates an HTML Template.
  • It initializes a SMTP connector.
  • It sends an email.

Why it is wrong ?

  1. Is doing more than One Thing.
  2. The unit test for that function is going to be huge and messy, why? E.g, the testing logic should consider the authentication and the welcome email process at the same time, when those process should be tested separately.
  3. It's easier to introduce errors, let's say we want to change the welcome email, we'll need to touch our function, so, making changes in process A for fixing process B doesn't make sense.
  4. If we try to do more than one thing, it would increase our function line numbers, losing readiness.
  5. More things to do, it's harder to debug.

Said that, how we can try to fix or reduce the issues listed above? Let's refactor our code.

def signup(email, password):
    new_user = create_user(
        email,
        password
    )
    authenticate(user)
    send_welcome_email(email)

def create_user(email, password):
    try:
        user = User.objects.create(
            email=email,
            password=password
        )
        return user
    except UserAlreadyExists:
        raise Exception('User exists')

def authenticate(user, password):
    auth_valid = authenticator(
        user=user,
        password=password
    )
    if auth_valid:
        user.last_login = datetime.now()
        user.save()

def send_welcome_email(email):
    email_msg = create_welcome_email_msg(email)
    send_email_to(
        email,
        email_msg
    )

def create_welcome_email_msg(email):
    msg_content = '<h2>Welcome {email}</h2>'.format(
        email=email
    ) 
    message = MIMEText(
        msg_content,
        'html'
    )
    message.update({
        'From': 'Sender Name <sender@server>',
        'To': email,
        'Cc': 'Receiver2 Name <receiver2@server>',
        'Subject': 'Account created'
    })
    msg_full = message.as_string()
    return msg_full

def send_email_to(email, email_msg):
    server = smtplib.SMTP('smtp.gmail.com:587')
    server.starttls()
    server.login(
        'sender@server.com',
        'senderpassword'
    )
    server.sendmail(
        'sender@server.com',
        [email],
        msg_full
    )
    server.quit()
Enter fullscreen mode Exit fullscreen mode

What we did ?

Much better, right?

  • We split up our create_user function into smaller functions, allowing us to separate the logic and adding a new different level of abstractions like signup function.
  • We applied the same principle (One Thing Rule) to the rest of the functions as well. So, readiness was improved, you can easily do unit testing and you can now change your welcome email without touching anything about user creation process.

Pro-tip:
So, how we can know when our function is not doing just one thing? the simplest way is if you can extract some logic from your function and putting it in a new one using a different logic name, most likely your function was doing more than one thing.

Level of abstraction

In order to ensure our functions are doing one thing, we should keep the same level of abstraction across all function.

In our past example (Code Preview 1), we could spot different levels of abstraction, we are authenticating the user using a
high level function authenticator(user=user,password=password) but at the same time, we're creating a HTML template from a string msg_content = '<h2>Welcome {email}</h2>'.format(email=email). This kind of function definition should be avoided.

Basically, we should be consistent at the moment we create our function. It also helps the reader to understand if our function is a high-level concept or it is a detailed process.

Sections within functions

Another way to spot if you function is doing more than one thing, if you can split it up by different sections grouping bunch of lines

def foo():
    # initialization
    # ...... several lines here
    # logic 1
    # ....... several lines here
    # Prepare return value
    # ....... several lines here 
Enter fullscreen mode Exit fullscreen mode

In this example, we are doing more than one thing at the same level, in order to avoid this, you can extract every section in other functions and then use them together, of course, it depends on the level of abstraction your function has.

Identation level

I encountered myself trying to understand functions with more than 2 levels of indentation. If you did as well, we can be agree
on the maximum level should be 2 as the worst case.

If you function have a lot of if/else blocks within other if/else blocks, for sure, your function is not doing One Thing. Let's see an example.

def buy_concert_ticket(user, ticket):

    ticket_price = ticket.price
    user_age = user.age

    if user.age >= 18:
        user_money = user.money
        if user_money >= ticket_price:
            seats = stadium.seats
            seat_available = None
            for seat in seats:
                if seat.is_available():
                    seat_available = seat
                    break

            if seat_available: 
                seat.owner = user
                user.money -= ticket.price
                print("Congratulations, you have a seat")
            else:
                print("There is not available seat")

        else:
            print("Sorry you don't have enough balance")
    else:
        print("You are not allowed to buy a ticket")
Enter fullscreen mode Exit fullscreen mode

So, these functions allow a user to buy a ticket, but before to complete the purchase, it verifies some conditions.
This function doesn't meet our previous rules:

  • It has more than 2 levels of indentation/blocks.
  • It does more than one thing.
  • It has several sections.
  • It difficult to read due to nested if/else blocks.

How to fix this?
1.- We should try to decrease the level of indentation by removing nested if/else blocks.

def buy_concert_ticket(user, ticket):

    ticket_price = ticket.price
    user_age = user.age
    user_money = user.money

    if not user.age >= 18:
        print("You are not allowed to buy a ticket")
        return 

    if not user_momey >= ticket_price:
        print("Sorry you don't have enough balance")
        return

    for seat in steats:
        if seat.is_available():
            seat.owner = user
            user.money -= ticket.price
            print("Congratulations, you have a seat")
            return

    print("There is no available seat")
    return
Enter fullscreen mode Exit fullscreen mode

You see, we were able to drop all the if/else blocks, so our max indentation level now is 2. Now, the function
has more readiness.

Pro-Tip: If you try to make simpler functions using the explained rules, you will never need to use an else again for function logic purposes.

2.- Let's identifies the things our function does and extract them to separated functions. The refactored code should look like this.

def buy_concert_ticket(user, ticket):

    if not user_has_legal_age(user):
        print("You are not allowed to buy a ticket")
        return

    if not user_has_enough_balance(user, ticket):
        print("Sorry you don't have enough balance")
        return

    available_seat = get_available_seat()

    if not available_seat:
        print("There is not available seat")
        return

    buy_seat(user, available_seat)

    return

def user_has_legal_age(user):
    user_age = user.age

    if not user.age >= 18:
        return False

    return True

def user_has_enough_balance(user, ticket):
    user_money = user.money
    ticket_price = ticket.price

    if user_momey >= ticket_price:
        return True

    return False

def get_available_seat():
    seats = stadium.seats
    for seat in seats:
        if seat.is_available():
            return seat

def buy_seat(user, seat):
    seat.owner = user
    user.money -= ticket.price
    print("Congratulations, you have a seat")
    return

Enter fullscreen mode Exit fullscreen mode

We were able to extract four new functions from our existing function, it means we were trying to do too much.

Now, our main function buy_concert_ticket can be read using "natural language". Of course, I used accurate names(we're going to talk about this later on) but having a small number of lines, just one level of extra indentation, helps a lot to achieve this result.

3.- Do you think we can't make our function smaller? or reduces sections? We do, let's see it.

def buy_concert_ticket(user, ticket):

    if not user_can_buy_a_ticket(user, ticket):
        return

    buy_available_seat(user)

    return

def user_can_buy_a_ticket(user, ticket):

    if not user_has_legal_age(user, ticket):
        print("You are not allowed to buy a ticket")
        return False

    if not user_has_enough_balance(user, ticket):
        print("Sorry you don't have enough balance")
        return False

    return True

def user_has_legal_age(user):
    user_age = user.age

    if not user.age >= 18:
        return False

    return True

def user_has_enough_balance(user, ticket):
    user_money = user.money
    ticket_price = ticket.price

    if user_momey >= ticket_price:
        return True

    return False

def buy_available_seat(user):
    available_seat = get_available_seat()

    if not available_seat:
        print("There is not available seat")

    buy_seat(user, available_seat)
    return

def get_available_seat():
    seats = stadium.seats
    for seat in seats:
        if seat.is_available():
            return seat


def buy_seat(user, seat):
    seat.owner = user
    user.money -= ticket.price
    print("Congratulations, you have a seat")
    return
Enter fullscreen mode Exit fullscreen mode

We could trim our main function a bit more, now we reduced it from 21 lines to just 4. We just applied the same principle,
try to abstract our logic and identifying sections/blocks in order to create new functions. This will become in an iterative process.

So, what we gained?

  • Anyone trying to read our function would take just seconds to understand what we were trying to do.
  • You can read the whole process just like a "top/down story".
  • Now, implementing unit test would be easier because we treated every step of buying ticket process as a single function.
  • If we want to add extra requirements in our process, identifying the function to change will be enough, for example: adding another restriction for a buyer, we just need to change our logic within user_can_buy_ticket() function without touching anything else.

Note: I know, doing this is time-consuming, but trust me, even when you can't iterate so many times due to deadlines, etc. Try to do it at least once. Always the first iteration brings good results.

That's all. In the next chapter, I'm going to talk more about how function parameters should be defined in order to improve readiness, giving other tips as well.

If you like my content or it was helpful, you can motivate me to write more content by buying me a coffee
Originally posted in my personal blog

Top comments (25)

Collapse
 
harshilpatel007 profile image
Harshil Patel

SOLID principles is great for writing testing friendly code..!!
Great post.
following_for_more_post()

Collapse
 
levivm profile image
Levi Velázquez

Glad you liked it.

I will post the next chapter soon.

Collapse
 
jprochazk profile image
Jan Procházka

Personally, I would put throw statements at the start of a function to avoid large indentation. It really improves readability.

Instead of

func doThing(x: int) {
    if(x <= 10) { 
        print(x)
    } else {
        throw Exception("x is too large!")
    }
}

it would be

func doThing(x: int) {
    if(x > 10) { 
        throw Exception("x is too large!")
    }

    print(x)
}
Collapse
 
peterwitham profile image
Peter Witham

If there is one rule that gets broken more than most it has to be the 'do one thing', been there and guilty of that. Then when you suffer at the hands of you're own code, you suddenly realize why it's a thing.

Collapse
 
cescquintero profile image
Francisco Quintero 🇨🇴

Great post. Functions should always be so small they can be easily tested and read!

Testing code becomes really easy when functions(even classes) just do one thing.

Reading code becomes extremely easy to read when functions just do one thing.

Another way to answer the question

how we can know when our function is not doing just one thing?

Is by telling what the function does. As soon as we describe and use the word "AND" that function is doing more than it should.

"my function sends info to third-party and updates the record" red flag.

Collapse
 
douglasbarbosadelima profile image
Douglas Barbosa de Lima

Good job! For me, the code legibility is the principal skill to great developers.

Collapse
 
levivm profile image
Levi Velázquez

Glad you liked it. Yep, that is the most valuable skill for sure.

Collapse
 
earroyoron profile image
Ernesto λrroyo • Edited

Great post: the code is an abstraction for people; we write code for people not for computers..

Collapse
 
kevinhch profile image
Kevin

Tabs or spaces? :)

Collapse
 
levivm profile image
Levi Velázquez • Edited

Sometimes depends on the language being used, sometimes depends on the person. I can't say that there is a Real/Definitive answer but my preference is spaces.

I always map my tab key to spaces. Why? I use python mostly and tabs may vary depending on the system, this could lead to an enormous amount of issues, a space is just one column. Of course, you need to learn how to navigate through the code independently of the selected method to avoid using right/left arrows.

If you use Javascript, this guide made by Airbnb suggest two spaces instead of hard tabs. I consider this a good style guide for ES6 (javascript).

Python == Spaces and so on.

If you share code with other developers, you must agree on a standard no matter your choice.

If you want to use tabs and you are comfortable with it, it's just ok. But, I'm an astronaut(space person ;) )

Collapse
 
ibrahimisad8 profile image
Ibrahim Isa

Does it matter :)

Collapse
 
omarjalil profile image
Jalil

Tabs of course

Collapse
 
jijii03 profile image
lincey.J

I always forgot about the "one thing" in every projects thanks for those tips!

Collapse
 
jasterix profile image
Jasterix

What's a best practice for ensuring that your code is easy to follow? I've seen Gists with so many helper functions and callbacks that it's hard to follow the thread of execution

Collapse
 
levivm profile image
Levi Velázquez

I'll create a post explaining the best way to do that.

But, functions definitions should be written in top-down schema based on his level of abstraction and importance.

Collapse
 
jasterix profile image
Jasterix

Looking forward to it

Collapse
 
codenutt profile image
Jared

Excellent article! I read about functional programming not too long ago, and have dedicated my code to try this "do one thing" approach and its made my life so much easier! Good work.