Hard-to-read code improvement notes

"Don't hate people, hate code."

Even so, does everyone have the experience of thinking like this more than once? What kind of code did Umi-sensei get angry with? And what should I do?

When I wrote such a code, I would like to look back on my code and actions and summarize it so that I can write a code that does not offend Professor Ohara. It's written in a straightforward manner for everyone, but I would appreciate it if you could forgive me.

What is hard-to-read code?

1. It's long enough to require scrolling to read a single process. The number of lines, 100 lines or more.
2. The conditional expression of the if statement is long.
3. There are many if statement branches.
4. There is an if statement in the if statement, and in it ... infinite loop is scary, isn't it?
5. Method names and variable names are abstract.

Isn't it like this?

What's wrong with writing hard-to-read code!

1. I don't want to read it first.
2. It's my job, so I have no choice but to read the code. It's also very concentrated.
3. Say "I don't know why" and go out for coffee tired. I have a pain in my eyes. And never look over again.
4. I want to implement it, but it takes time to understand the code, and the extra time is wasted.
5. I'm scared when I try to implement it. And I don't want to get involved in this code again.
6. The trouble is that this phenomenon is the same for the person who wrote the code.
7. I don't know where to hit this anger if the person who wrote it is transferred to another team (Idadan ... Joudandesu? Joudan ...)

The man-hours that would otherwise be small are inflated due to the extra work of "understanding complex, mysterious and difficult-to-read code," resulting in lost opportunities. Furthermore, ** added functions and existing functions may cause a chemical reaction to create new bugs **. And reread the code again to fix the bug ... just ** negative spiral! !! ** **

If you write as a personal hobby, it would be nice if you could satisfy yourself, but if you develop it as a team or if the customer manages it after delivery, the crime of hard-to-read code is very deep.

How can I make hard-to-read code?

For example, in a smartphone game, there are quests, but in the process of starting a quest,

--Are you trying to start a quest during the period? -Are you clearing the conditions to play this quest? ――What kind of enemies will come out? ――What kind of reward will you get if you defeat the enemy? -Is the quest member cheating? -Is the item you bring to the quest fraudulent?

(I think there are others depending on the game). If you write these various processes in one method as they are, you can make code that is difficult to read. Actually, it is ** yes ** to write these processes in one method (because you can make it a quest controller role and call this with API), but it is dangerous to write it too obediently.

How to write code that is not hard to read?

Here's what I'm currently doing to fix my code so that it doesn't become a negative legacy.

Create a method for each function

The code is long because it implements various features. So, don't write the code as it is, but for each function

-Is it a quest during the period? => check_quest_period () -Do you meet the quest play conditions? => is_playable_quest () ――What kind of enemies will appear? => select_enemy ()

Create a method like this and call it.

By doing this, for example, if you need to "add a new specification to the enemy's lottery method", you can start working quickly because you only have to look at select_enemy () without reading other processes. If select_enemy () is long, it is highly likely that multiple functions are implemented as they are, so let's divide them into methods for each function.

Make an if statement a method

The condition of the if statement is long and there are many branches, depending on the specifications. In that case, if you make the if statement a method, you can understand what you are doing just by looking at the method name, which helps to decipher the if statement.

For example, quest processing

quest_start.py

class QuestStartManager(object):
    @classmethod
    def quest_start(cls, player, quest, now_datetime)
        #Check if the quest is in progress
        if quest.start_datetime > now_datetime or now_datetime > quest.end_datetime:
           raise OutOfPeriodError

        #Check if the quest play conditions are met
        if player.level < quest.need_level or quest.need_clear_quest_id not in player.clear_quest_ids or (Omitted because it cannot be written):
           raise PlayerQuestStatusIsNotPlayableError

(Hereafter, the code is omitted)

I don't want to read it anymore just by looking at this ... it's just a few lines! if statement ... a scary child!

quest_start

class QuestStartManager(object):
    @classmethod
    def quest_start(cls, player, quest, now_datetime):
        check_quest_period(quest, now_datetime)
        is_playable_quest(player, quest)

How about this? Isn't it easier to read than the if statement is written directly? An if statement is written in each method, but you can imagine from the method name "It's a period check" or "Is it checking if you can play the quest?"

Write specific method names and variable names

I think that a short code like the one below is acceptable (this is just an example, and I actually write it as try ... except. I'll excuse it just in case).

quest.py

class Quest(models.Model):
    @classmethod
    def get_quest(cls, quest_id):
        obj = cls.objects.get(quest_id)
        if obj:
            return obj
        else:
            return None

But what if you write this variable name obj in a long process? If this "obj" appears in many places as you scroll, you will be worried. ** "Which obj is it !!" ** You'll want to put in a tsukkomi.

If the method name is just get_obj () or update_obj (), you won't know which obj you're trying to get or update, and you'll doubt the reliability of this code.

When deciding on a name, ** the name is unusually long (there are multiple verbs) ** or ** I'm thinking of a name, but if I'm worried about what kind of name fits **, it's almost ** overloaded with functions ** So let's cut out the function.

I want to draw a comment in some detail! !! If so, let's stop

Syndromes that you want to write in detail often occur. I analyze that this is because the psychological state that other people in charge will not understand the contents of the code I wrote is exposed (I am).

It's ridiculous to leave it as it is! !!

Let's consider whether it is possible to divide it into methods for each function.

Design pattern

** It's a complicated process, so it can't be helped if it becomes difficult to read **. In that case, consider whether you can use a design pattern. There may be a solution there. so. If it's a design pattern.

Include refactoring man-hours in the quote

Recall the exams you had when you were a student. Didn't the teacher say "Review"? No matter how good the students are, they will make mistakes. It was ** review ** to prevent that.

This is the same for code implementations. Sometimes I thought it was good when I designed and implemented it, but as the work progressed, it turned into something bad. Even the person who wrote that it was terrible may have a code such as "What is this? What is this?"

Let's tell the director the man-hours for reviewing the estimated man-hours for mounting.

I think that "just move without problems" is allowed only within the scope of personal hobbies. (Tell yourself (bloody tears)) Extensions and bug fixes will occur as long as the service continues. ** Can you say that the code will take care of you forever? ** **

** Important review. ** **

Let's write test code

If you can write easy-to-read code from the beginning, you may not need this kind of work ... No, no matter what the test, rework will occur (I want to believe).

--Separate the process into methods. --Make the processing of if statement a method.

And, if you modify the processing that was working until now, a bug may occur due to that.

But it would be safe to have a test code. When I run the test, I get an error! !! And if the test passes, you can have the debugger debug it with confidence.

The unit test is also covered here posted on our Advent Calendar, so please refer to it as well.

Until you can automate unit tests

Also, my post is also w About the test

Finally

After the release, we have to implement the measures, so this effort becomes quite difficult. Also, code that was easy to read before its release tends to become harder to read over time. If you want to provide the service for a long time and soundly, I think you should schedule the refactoring time and unit test implementation time in man-hours even after the release. And let's improve ** gradually **.

If you deliver the system to a customer and need to fix a bug, the hard-to-read code will result in an inaccurate estimate (I'm afraid I'd like to save some extra effort, but the customer will surely disagree. When that happens, the spirit of the sales staff is gaga ...). Hard-to-read code is really sinful.

I would like to work on the above so that the code I wrote will at least be "not difficult to read".