How Wrapping an Expression as a Function Can Be Clean Code

clean codecode-reviewsteamwork

Another programmer just started working in our team and submitted a patch. What required was to have something that compares and check a couple of conditions and set a property based on the outcome. The patch, in essence, is a class that implements the requirement as something like this:

class Processor {
    A a;
    B b;

    public function process(A a, B b) {
        this.a = a;
        this.b = b;

        if (false == this.isVerified() && this.equalNames()) {
            this.setVerified();
        }
    }

    private function equalNames(): bool {
        return this.a.name() == this.b.name();
    }

    private function isVerified(): bool {
        return this.a.isVerified();
    }

    private function setVerified() {
        this.a.setVerified(true);
    }
}

The actual code probably had slightly more detail but the pseudo code is detail enough, I think. Basically, what happen when we reviewed the code we were dumbstruck. To make matter worse, he couldn't give the explanation beyond that his code is "Clean Code" and telling us to "Look how clean it is!" and read the book. Actually, someone pointed out that to utilize his code, one would need to instantiate that class first, then call process() to which his reply was "What's wrong with that?"

Since we needed to get the job done, one of us proposed to replace that class with a method within AService class:

public function verifyA(A a, B b) {
    if (!a.isVerified() && a.name() == b.name()) {
        a.setVerified(true);
    }
}

That is what finally got merged. Now, I feel that I want to know if there any truth behind wrapping a single logical expression in a function because I don't want him to feel being ignored (in his code review he always put comment on everyone's if statements to put them in function, and so far no one oblige). Also to make him or we understand how to explain whatever the right way is.

I haven't read the Clean Code book. I do read Uncle Bob's blog and understand putting business logic in functions and. My question is more specific to putting a single logical expression in function and reference to the book if any. The secondary question is how to handle debating this issue as it has come up a few times and affecting the team's dynamics.

Best Answer

What constitutes "clean code" always depends on context. The exact same lines of code could be either perfectly clean or hideously over-complicated depending on the requirements of the application.

The design in question allows you to use dependency injection to substitute implementations of the process without affecting clients. So this is clean and SOLID code if you have this requirement. If you don't have such a requirement, then it is over-complicated and violating the YAGNI and KISS principles.

Since neither you or the developer could explain the reasoning for the design I think the second option is the most likely. Probably the developer have seen this design in a context where it makes sense, but applied it in a context where it didn't make sense.

Related Topic