“Stateless code reviews” is a term of my own invention and I will explain in detail what it means. If you don’t know what “stateless protocol” or what “code review” is, you should probably read some other stuff first, try wikipedia on this and this!

Stateless as explained in wikipedia is something that “no information is retained” and I am using this word to explain that there is no need of application knowledge (or even better business knowledge) to make a good code review.

Deep knowledge of the language

Each reviewer wants to make a respectful code review and bounce a piece of code back to the developer with no inaccuracies on his bouncing comments. In order to achieve this the reviewer needs to have an excellent knowledge of the language that the code is written in.

Translating the same logic in better writing (custody)

To make this even more clear (or more difficult for people not thinking like I do) imagine what a compiler does: Translates code into machine language to help the computer understand it.

I am talking about another translation which happens on developing-code level, that makes the code more readable and more clear for the team using it. Confused? Let’s try a real life example in PHP code!

<h3>Recieves SMS notifications</h3>
<ul class="nav subscriptions">
    <li class="subscription">
    <?php
        if (!$user->phonenumberIsMobile() && $user->hasSmsNotifications()) {
            $isDisabled = true;
        } else $isDisabled = false;

        if ($user->hasSmsNotifications()) {
            $checked = true;
        } else $checked = false;
    ?>
    <?=
        $form_helper->checkbox(
            sprintf('smsNotifications'), $checked,
            $options = [
                'checkbox_id' => 'smsNotifications',
                'label' => 'Allow SMS messages to my phone',
                'input_disabled' => "disabled='disabled'"
            ]
            , $isDisabled

        );
    ?>
    </li>
</ul>

Nice code right? It looks OK-ish on the first sight but let me begin with my stateless comments.

if (!$user->phonenumberIsMobile() && $user->hasSmsNotifications()) {
    $isDisabled = true;
} else $isDisabled = false;

All this 3 lines is a complete disaster as it could be written in only one:

$isDisabled = !$user->phonenumberIsMobile() && $user->hasSmsNotifications();

And of course the same applies for the next 3 lines of code

if ($user->hasSmsNotifications()) {
    $checked = true;
} else $checked = false;

That would look better like this

$checked = $user->hasSmsNotifications();

So we just took 6 lines of code and we made them 2 lines that are read with more ease, right? And what follows? End of PHP script, and begin of PHP script with some whitespace in it – whitespace that it’s not visible in the browser as it’s between tags – removed!

Our resulting code looks like this now:

<h3>Recieves SMS notifications</h3>
<ul class="nav subscriptions">
    <li class="subscription">
    <?php
        $isDisabled = !$user->phonenumberIsMobile() && $user->hasSmsNotifications();
        $checked = $user->hasSmsNotifications();
         $form_helper->checkbox(
                sprintf('smsNotifications'), $checked,
                $options = [
                    'checkbox_id' => 'smsNotifications',
                    'label' => 'Allow SMS messages to my phone',
                    'input_disabled' => "disabled='disabled'"
                ]
                , $isDisabled

            );
        ?>
    </li>
</ul>

Is this good enough? No, of course it’s not! We can go on and on with this code. The sprintf method takes 2 parameters at least, when it will not break execution with only one but the truth is that:

sprintf('smsNotifications') === 'smsNotifications'

So we don’t need to wrap the string in sprintf at all! Also I never like to have some parameters of a method in the same line if not all of them are in the same line, and I hit an enter before $checked to make sure it’s on the next line.. Wait.. This is only used once, why have it as a variable and make the developer remember more and more? So let’s replace $checked with the equivalent code assigned in the variable – also I will do the same with $isDisabled.

<h3>Recieves SMS notifications</h3>
<ul class="nav subscriptions">
    <li class="subscription">
    <?php
        $form_helper->checkbox(
            'smsNotifications', 
            $user->hasSmsNotifications(),
            $options = [
                'checkbox_id' => 'smsNotifications',
                'label' => 'Allow SMS messages to my phone',
                'input_disabled' => "disabled='disabled'"
            ],
            !$user->phonenumberIsMobile() && $user->hasSmsNotifications()
        );
    ?>
    </li>
</ul>

How much better does this code look now? I think much! Can it be even better? Probably yes, but we will not dive into it in this article!

Stateless

The example that we analyzed was a stateless code review as we didn’t talk about the logic at all, I don’t even know what this code does, probably a checkbox to give permission on an application to send SMS notifications, but who cares?

Automatic implementation

I did a quick search on the net for an automatic tool that will make this kind of code reviews and give suggestions but I came up only with code sniffer like tests and linters. I believe it’s quite hard to do this automatically as it is based on the culture that the architect of each project wants to promote. It is very tightly coupled with the conventions of an application and of course the language used.

I personally think that in a couple of years this will be a must in most companies and they will outsource “stateless code reviews” in order to avoid spending time of their own on something not business specific! Companies will try to make it automatic but I think that this will be very very difficult considering the diversity of code around us.

Keep coding!

Leave a Reply

We are a software house!

A place that we gather all together to build, test and ship software for high demanding clients.

Our headquarters

Ipirou 16
Drama, 66100
Greece

T: +30 2521 105247
E: [email protected]