Shafiya's Blog

Shafiya's Blog

Writing Clean Code in Python

How do you measure if your code is good? Aside from making sure your code is bug-free, your code need to be clean. Code is considered clean if it can be understood easily. "I can understand my code easily since I am the one who wrote it". Leave the code for two months, you probably forget what you wrote. Writing clean code doesn't mean you can remember it directly, but it will make you easier to understand once you read it later.

Take a look at this example:

ymdstr = datetime.date.today().strftime("%y-%m-%d")

You probably know this variable is used to store today's date in YY-MM-DD. What about your teammate? If your teammate see it in his/her first time, your teammate most likely will wonder "What is ymdstr?". He/she can guess, like "Oh, it's year, month, day in string", but that would make he/she thinks hard, right?

Not only helping you understand the code later if you want to look at it in the future, it saves you from getting blamed by your teammate too 😂. If you want to know how to write clean code in Python, here some examples:

Use meaningful name for variables

Here is one of example taken from my PPL project:

status = request.query_params.get("status")

You can see that status variable is to store value of status' value from query params. If the URL is ?status=Done, then status variable will store Done as its value.

Use meaningful name for functions

class EmailRegisteredLoginValidator:
    def __init__(self, account):
        self.account = account

    def validate(self, email):
        try:
            self.account.objects.get(email=email)
        except self.account.DoesNotExist:
            message = _("Email is not registered yet.")
            raise ValidationError(message, code="authorization")

Take a look at validate function. Based on its name and its parameter, we can infer quickly that validate tries to validate an email. Now, if we read more into the code, apparently our guess is right, since validate will check if email is already registered or not.

Avoid mental mapping

I once made mistake like this:

user = self.request.user.pk

You might think something like this:

  1. Since the variable's name is user, it probably stores about user's information.
  2. If you see self.request.user.pk, you might know it will store user's primary key (pk = primary key).
  3. If you remember database course, user's primary key might be an ID. You can infer that user variable stores user's ID.

It is not hard to guess the value of user variable, but we need to at least guess three things: what value does user store?, what is pk?, what is the primary key of user?

We can make it better:

user_id = self.request.user.id

Now, we know that user_id stores user's ID directly based on its variable name. If we see the value, we can see that it tries to get user.id

Functions should do one thing

Here is one of bad example that I created:

class LoginSerializer(serializers.Serializer):
    email = serializers.EmailField(required=False)
    password = serializers.CharField(required=False)

    def validate(self, attrs):
        email = attrs.get("email")
        password = attrs.get("password")

        if password is None or password == "":
            message = _("Password cannot be empty.")
            raise serializers.ValidationError(message, code="blank")

        if email is None or email == "":
            message = _("Email cannot be empty.")
            raise serializers.ValidationError(message, code="blank")

        try:
            Account.objects.get(email=email)
        except Account.DoesNotExist:
            msg = _("Email is not registered yet.")
            raise serializers.ValidationError(msg, code="authorization")

        user = authenticate(
            request=self.context.get("request"), email=email.lower(), password=password
        )

        if not user:
            msg = _("Password is wrong.")
            raise serializers.ValidationError(msg, code="authorization")

        return attrs

Aside from long lines of code that will make you confused, the function is too big when it come to the responsibilities. validate function tries to do these things:

  1. Validate if password is not empty
  2. Validate if email is not empty
  3. Validate if email is registered
  4. Validate if password and email match

How can we improve this? We are going to create validators class (class that get assigned to check password, email separately) and validate tries to call those classes. Here is the example:

class LoginSerializer(serializers.Serializer):
    email = serializers.EmailField(required=False)
    password = serializers.CharField(required=False)

    def validate(self, attrs):
        email = attrs.get("email")
        password = attrs.get("password")

        EmptyPasswordLoginValidator().validate(password)
        EmptyEmailLoginValidator().validate(email)
        EmailRegisteredLoginValidator(Account).validate(email)
        CheckPasswordLoginValidator(
            request=self.context.get("request"), email=email.lower(), password=password
        ).validate()

        return attrs
class EmptyPasswordLoginValidator:
    def validate(self, password):
        if password is None or password == "":
            message = _("Password cannot be empty.")
            raise ValidationError(message, code="blank")

Aside from making the code shorter, you can see that it is clear what those classes are trying to do. The validate function doesn't handle the logic directly and it takes less responsibility.

Error handling

When it comes to error handling, it prefers to raise Exceptions to return something. Here is an example of raise an error:

class EmptyEmailLoginValidator:
    def validate(self, email):
        if email is None or email == "":
            message = _("Email cannot be empty.")
            raise ValidationError(message, code="blank")

Why it is preferred to raise an error rather than return something? If we return something, here is the example:

def validate(email):
    if email is None or email == "":
        return False

email = validate(email)
if email is False:
    return "Email cannot be empty"

Return something means that we have to check the value using if-else again which makes our code less clean.

You can use try catch for error handling as well. It is preferred to specify what error, rather than leaving except as it is:

class EmailRegisteredLoginValidator:
    def __init__(self, account):
        self.account = account

    def validate(self, email):
        try:
            self.account.objects.get(email=email)
        except self.account.DoesNotExist:
            message = _("Email is not registered yet.")
            raise ValidationError(message, code="authorization")

For more guide on writing clean code in Python, you can see this reference.

Conventions

Each language has unique style guide to make its code clean. For example: in naming variables, rules in Python might be different with JavaScript, so you need to check on the rules. Python has standardized style guide, called PEP8

Here are few of them:

  • class name should use CapWords convention. We use RiwayatView, not riwayatview for class name

  • variable in function should be lowercase We use user, not User for variable name

"There's too many of them. How do I suppose memorize this style guide?" Worry not, you don't need to memorize since you can use a helper tool called linter. Linter helps you to avoid conventions errors.

We use flake8 for our linter.

First, you need to install it first:

pip install flake8

To use, you can call it directly:

flake8

If flake8 found style error, it will report like this:

image.png

Tips (that I have just learned about Python)

  • You want to use value in dictionary, but there is a case where the key of value doesn't exist. Example: I want to use status key in query_params. Usually, I would do like this:
status_value = None

if status in request.query_params:
        status_value = request.query_params["status"]

I have just learned that you can use get:

status = request.query_params.get("status")

Using get, if status key doesn't exist, it will have default of None. If it does exist, it will access the value of status. So you don't need another if-else for that :)

  • You can unpack iterables and assign them to named variables. Usually, in order to access tuple, I do this:
# this return tuple
self.token = Token.objects.get_or_create(user=self.account)

# to get token
self.access_token = self.token[0]

You can make it more readable like this (without accessing index):

self.access_token, created = Token.objects.get_or_create(user=self.account)

Conclusion

  • Clean code helps you and your teammates
  • There are steps to implement clean code: meaningful variable name, meaningful function name, avoid mental mapping, functions should do one thing, error handling, and many more.
  • Each language has conventions. Python has PEP8. You don't need to memorize PEP8, just install linter.
  • If you dig into the language, you will find things (functions, etc) that make your code more readable.
 
Share this