Shafiya's Blog

Shafiya's Blog

Applying Design Principle in Error Handling

During TDD phase, you will find out that your code need to be refactored. How do you know if your code should be refactored or not? Let's take a look at one of example:

class CreatePTSerializer(PTSerializer):
    def validate(self, data):
        if data.get("kode_kbli"):
            kode_kbli = data.get("kode_kbli")
            if not re.match(r"^\d+$", kode_kbli):
                raise ValidationError(_("Kode KBLI hanya boleh mengandung angka!"))
            if len(kode_kbli) != 5:
                raise ValidationError(_("Kode KBLI harus terdiri dari 5 angka!"))

        return data

Do you find anything that bother you? It has nested conditionals, which makes me confused😅. After, we are checking if the data has kode_kbli or not, there are another conditionals. The first one... it uses Regex😲😲😲. You may wonder "What does that Regex checks?". After finding out it checks if all characters should be numbers, there is another condition again. Glad the last one does only checks length.

What happens if in the future we add another validation? For example, kode_kbli should be in range from 1000 to 5000. We need another condition! You wonder, it seems that this code is not right. "This is considered as code smells, right?".

Yes, in fact, if you remember about Design Principles (SOLID), it violates "O" principle, or if you want to know the detail, it is called Open/Closed Principle. The code above violates Open/Closed Principle.

Open/Closed Principle

Open/Closed Principle, one of design principles, proposed by Uncle Bob, states this meaning:

Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.

I told that the example above violates this principle. Why is that? It is because if we need to add more conditions, we will modify the validate function.

So, what is the solution? One simple thing to refactor is that we could try is to make a separate validator class for each condition. Here is one of example that we made:

class CreatePTSerializer(PTSerializer):
    def validate(self, data):
        if data.get("kode_kbli"):
            kode_kbli = data.get("kode_kbli")
            NumericKodeKbliValidator().validate(kode_kbli)
            LengthKodeKBLIValidator().validate(kode_kbli)

        return data

class NumericKodeKbliValidator():
    def validate(self, kode_kbli):
        if not re.match(r"^\d+$", kode_kbli):
            raise ValidationError(_("Kode KBLI hanya boleh mengandung angka!"))


class LengthKodeKBLIValidator:
    def validate(self, kode_kbli):
        if len(kode_kbli) != 5:
            raise ValidationError(_("Kode KBLI harus terdiri dari 5 angka!"))

Wow, so cool! It still raises the error even though we refactored the code to use classes! Cool.gif

One thing that we should emphasize is that, even we refactored this code, it won't break our features. Why is that? Oh, we use TDD. Yep, TDD helps us to become more confident in refactoring. If we refactored our code wrong, it will make the tests fail. Since we know that the tests are right and our implementation before refactoring is right too, it means that we do something wrong in our refactored code. Now you know why TDD helps us a lot.

Okay, what else we can improve? Apparently, one of solution that is proposed from Open/Closed Principle is to promote using interfaces to apply close for modifications. By using interfaces, the detail of the implementations is freely, depends on the class that wants to implement the interface.

So, how could I make abstractions based on these two class? Glad it is very simple. Here is the diagram:

refactor.png

As you can see, we could make interface of PTValidator, where the implementation is freely based on the class needs.

In Python, we could implement like this:

class PTValidator:
    def validate(self, kode_kbli):
        raise NotImplementedError


class NumericKodeKbliValidator(PTValidator):
    def validate(self, kode_kbli):
        if not re.match(r"^\d+$", kode_kbli):
            raise ValidationError(_("Kode KBLI hanya boleh mengandung angka!"))


class LengthKodeKBLIValidator(PTValidator):
    def validate(self, kode_kbli):
        if len(kode_kbli) != 5:
            raise ValidationError(_("Kode KBLI harus terdiri dari 5 angka!"))

Unlike Java where it is supported to create interface, we need to do a little bit of duck typing here. Since in validate function of the interface will be implemented in the classes, we can make the function to raise NotImplementedError.

If you see it clearly, not only it applies Open/Closed Principle, our implementation also uses Single Responsibility Principle. Well, do you remember that?

Single Responsibility Principle

Single Responsibility Principle states this:

A class should have one, and only one, reason to change.

In the previous example, where there are many if-else, we need to change things in class CreatePTSerializer every time we need to add validations based on the requirements. Class CreatePTSerializer doesn't only create objects, but it also tries to validate the input too. Well, this is not good. We need to delegate the validations to others. Therefore, by refactoring the code like above, we are making sure that class CreatePTSerializer doesn't validate the condition directly, only raising error from delegated classes.

From classes that we made, we can see that the class only validates one condition, or what we can say, only has one responsibility. If we have more validations, then we are going to make another class that has responsiblity based on new validation. Therefore, we don't realize that our implementation also adheres Single Responsibility Principle. Great!

If you ask, can we apply design pattern here? Well, I am not sure, but I am already satisfied with the code. What we are doing is that we are fixing code that doesn't adhere Design Principles. Design Patterns are patterns that people have found it earlier. It should be remembered that Design Pattern can be applied if only the pattern matches our conditions. Don't try to use design pattern if we force it, like "We should use strategy pattern here!". Use it wisely, based on problems that we have.

 
Share this