Code readability and debugging

Question

Sometimes, I have a hard time deciding between two good code traits: debuggability and readability. The snippets below are an oversimplification, but they illustrate my pain.

Example 1:

if(isSomethingThere(somethingParam)
  || isTheOtherThingThere(otherParam)
  || isWhatever(whatever))
{
  // some logic
}

Example 2:

bool isSomethingThere = isSomethingThere(somethingParam);
bool isTheOtherThingThere = isTheOtherThingThere(otherParam);
bool isWhatever = isWhatever(whatever);

if(isSomethingThere || isTheOtherThingThere || isWhatever)
{
  // some logic
}

To me, Example 1 is an easy-to-read code, but a pain in the rear to debug. This code is a nightmare to debug because the debugger won't tell me what the return value of each method is. Moreover, the execution won't reach, for example, the isWhatever() method if any of the previous functions returns false (short circuit), and it is another extra information I need to keep in my mind when debugging (which is by default a heavy cognitive task).

To get the return values I need to either:

• Add some extra code. See Example 2. On the plus side, I can stop the execution at the if and see clearly what the situation is. On the minus side, it forces me to redeploy the code (testability testability of the service is another story rant, but luckily I can deploy code with 2-5 minutes of work)
• Step into every method and see what is happening there

All of this makes me wonder if

• Is there a good balance between readable code and debuggable code, or do we sacrifice some part of one of them (debuggability) for the other (readability)?
• What can be considered good (clean?) code in this case?
• Am I using the debugger effectively? I mean extending further my knowledge basically solves the problem I described here.

Answer 1

is there a good balance between readable code and debuggable code

Yes there is. As you wrote by yourself in a comment, example 2 is both - debuggable and readable^*. The real drawback is not decreased readability - it is what you wrote here:

On the minus side, it forces me to re-deploy the code (testability of the service is another story rant, but luckily I can deploy code with 2-5 minutes of work)

The cure for this are more automated tests you can run locally - so after changing the code to introduce boolean helper variable, you should be able to rerun and debug the code in 5 to 15 seconds, and not have to wait 5 minutes for it. 5 minutes is an interruption long enough to force a context switch in my thoughts - so I would recommend to work on reducing the required time from changing small pieces in the code to the point where the program reaches a break point at that code section.

Let me add a note about the missing short-circuit evaluation of example 2: to bring example 1 into a form where you can easily set break points, inspect the intermediate results one-by-one and still use short-circuit evaluation, you should write

bool found = isSomethingThere(somethingParam);
found = found || isTheOtherThingThere(otherParam);
found = found || isWhatever(whatever);

if(found)
 ...

_{(Of course, you may shorten this by using ||= instead, if your language allows it, but this is quite unimportant here.)}

If one considers this as less readable than example 1 and 2 is probably a matter of taste, but at least now it is 100% semantically equivalent to example 1, which example 2 is not.

_{* I am assuming you do not have a debugger at hand which can display return values of function calls in an expression directly, otherwise you would have already considered example 1 to be "debuggable".}

Answer 2

IMO you're missing the actual solution: A dedicated, descriptive method.
What you are encountering is an issue with 'Cognitive load', which you want to reduce. Lower cognitive load = Easier to understand (and from that often follows easier debugging).

Those three checks together test something. Maybe its a product and you want to check isActive + hasCompleteData + hasOneImage to determine wether or not a product can be displayed.

That would result in something like so:

if (isProductDisplayable($product)) {
  // some logic like building the template
}

/*
 * Maybe
 * other
 * code
 * here
 *
 */

function isProductDisplayable(ProductInterface $product): bool
{
    return isActive($product)
        && hasCompleteData($product)
        && hasOneImage($product)
    ;
}

The improvement here is the descriptive name of the methods combined. You now only need to read isProductDisplayable to know what we're testing. You'll likely dont care about the how, you just need to know we check if its displayable. This method can be expanded with new checks, you could also use early returns or slightly more complex checks without running against code clutter issues.

And if you care for the how or when you need to debug, only then you need to read the details of it. You now have a single method, which you can access with your debugger, or isolate in test class. You can now use your breakpoints per line, or place all three in a debug logger, or whatever you're preferred method is.

This way of implementing logic helps with de bugging because:

• It's simpler to understand what is going on
  • this makes it easier to find bugs
  • this avoids bugs beforehand
• This gives dedicated access to your logic
  • you now have one point to test/debug
  • You can single out this one function instead of having to test a complexer one
  • You can create unittest and lock the behaviour, avoiding bugs down the line.

---

This also helps with Single Responsibility. A piece of code should only have one reason for change. In your code thats not happening, because (mixing my example with your code) it checks of the product can be displayed, and then do something to display it. Thats two things, which isnt "allowed".

By splitting it in two each method does one trick. And you have the added benefit that this code is now also reusable if one wishes.

Answer 3

Your second example changed the meaning of the code. Part 2 and 3 of the “if” are executed even if part 1 is true, which can be anywhere from harmless to inefficient to disastrous. Another answer showed ways around this.

There are no prices for “longest possible if statement” and “most characters fit on a line”. Assigning part of an expression to a well-named variable can save you a comment and make code more readable while easier to debug at the same time. Especially if a condition isn’t just a call to a well-named function.

Code is easier to debug when you write it down in a simpler way and break it up in parts. That usually makes it more readable and often makes it more efficient.

Answer 4

Is there a good balance between readable code and debuggable code, or do we sacrifice some part of one of them (debuggability) for the other (readability)?

We used to have to type line numbers. Now the IDE counts the lines for us. Today, over one liners, I prefer fluffy code full of white space that shows structure and puts each call on a unique line because stack traces don't show you column numbers (well not in every language anyway).

We've always had to compromise between optimizing for either activity.

What can be considered good (clean?) code in this case?

Ask your team. This is what a code review is for. Strangers on the internet have about as much authority in the conference room as a flipped coin. Ask the people who have to live with it.

Do you think I don't use all the features of the debugger? I mean extending further my knowledge basically solves the problem I described here.

Does your team? If you discover a wonderful workaround for your debugging problem make sure you share it with the team before assuming they'll be fine with example 1. And if they haven't shared it with you, go bug them.

Answer 5

Is there a good balance between readable code and debuggable code, or do we sacrifice some part of one of them (debuggability) for the other (readability)?

The context matters, and your priorities (along with the team) matter too.

For example, #2 flags can be troublesome if isTheOtherThingThere or isWhatEver do heavy calculus. If they do, it would be reasonable to code if/else statements instead or, why not, leave a log trace from each method for debugging purposes.¹

Pragmatism is a good way to approach the dilemma. Whatever makes you productive is a good solution too. As @DocBrown suggests, your time (and your team's time) is valuable.

Note that, I have referred the team a couple of times. When it comes to the code we write, we are terrible reviewers and evaluators. Of course, my code is readable! And clean! Is self-evident... Right?

But you know what? Coding is a social act. We make readable code for others, we make clean code for others to understand what we do and how. So, in case of doubts, ask collaborators (teammates, coworkers, community) but remember who deals with the code 40h a week.

---

^{1: Logs can give you a broader vision of the execution context. They offer this vision without having to remember countless conditions and variables. For example, knowing if IsWhatever is false might not be as important as knowing the reasons for whatever to be in its current state and cause the false. If you know that the execution will fail, let it fail quickly! and leave logs to tell you the story.}

Answer 6

This is a great question because readability and debuggability are both very important properties. But while readability tend to follow quite general principles, debuggability very much depends on the particular debugging tools you have available. For this reason, debuggability is often glossed over, since it is harder to say something general about.

First, you should familiarize yourself with what debugging tools you actually have available. It may be more powerful than you think. Many debuggers allow you to inspect intermediate values even if they are never stored in a variable. This may not be quite as convenient as just inspecting the value a variable, but since code is read a lot more than it is actively debugged (hopefully!) it is worth prioritizing readability.

So in short code should be easy to read and it should be possible to debug, given full knowledge of the best tools. But you shouldn't compromise on readability to make debugging more convenient.

If you feel that debuggability force you to compromise on readability, I suggest you look into what alternative debugging tools/IDE's are available.

Answer 7

Where you have a multi-operand test like this together with wordy function names, my inclination (heaven forgive me...) would be:

bool b1 = isSomethingThere(somethingParam);
bool b2 = isTheOtherThingThere(otherParam);
bool b3 = isWhatever(whatever);

if (b1 || b2 || b3) {
  // some logic
}

This is one of the limited cases in which I'd tolerate variable names which are undescriptive, except that the numbering expresses that they are each part of a set, and they are immediately co-located with the lines which actually define them.

Effectively it's just a small device for allowing the series of names to be expressed on the vertical axis above the block header, and give you the breakpoints.

Sometimes an explosion of words in one place can be a sign of something else not quite right, either with the naming or the logical design.

If the logic within the if-block is trivial whereas the conditions which trigger it become a complicated expression of ANDs and ORs, it might be worth repeating the logic multiple times just so that you can separate the various cases which trigger it.

Alternatively, create an intermediary variable, which can be set from multiple places, but then finally triggers one block of common code:

bool performCommon = false; 

{
    bool b1 = isSomethingThere(somethingParam);
    bool b2 = isTheOtherThingThere(otherParam);
    bool b3 = isWhatever(whatever);
    bool b4 = isAnother(another);

    if (b1 || b2) { 
        performCommon = true;
    }

    if (b3 && b4) { 
        performCommon = true;
    }
}

if (performCommon) {
  // some longer logic worthy of avoiding duplication in the previous if-blocks.
}

The exact ordering and naming of everything might be a matter for further judgment, but you see the idea.

Ultimately, when names are so long to begin with, and if they have to be, then its difficult to avoid breaking things out over multiple lines, and using codes as aliases for the full names already given in the same locality.

Answer 8

One of the primary motivations you should have as a developer is to develop “maintainable” code.

The quest for simple elegant code doesn’t always align with comprehension and maintainability.

An OR test,vs an AND test, doesn’t require all of the functions to execute as others have stated.

As a maintainer or future consumer I would be asking why is the test an OR in the first place when this code becomes the focus of a debugging review.

The way the code is written doesn’t help me understand this. This is where good comments that aid comprehension come in and IMHO are required.

As others have said the IDE should help with debugging. Sometimes making assignments before conditional statements is good practice and vastly improves debugability. In many compilable languages (C, C#, Java etc.) assignments and tests can, as a side effect of compiler optimisation, be blended together in the most efficient way by the compiler so how the code is written may not be what the resulting executable looks like.

Answer 9

Production quality code will need room to grow

The code in your first example puts several method calls all within the same line. In my experience this style of coding would not survive all the way to production, as there is usually a need to put "goo" in between (e.g. logging, performance counters, computation of parameters, exception handling).

What I would suggest is extracting the logic to its own method so you can keep the goo separate from the main program. If it's in its own method then you can use early return to simplify the logic as well. And by using early return you make it very clear that you are using short-circuit logic (something you seemed to have forgotten in your second example).

Here is a short sample including some additional goo that I made up:

bool CanDoSomething(some inputs)
{
    var somethingParam = inputs.A + inputs.B;
    if (!isSomethingThere(somethingParam))
    {
        _log.Debug("Unable to do X because Y isn't there.");
        return false;
    }

    var otherParam = inputs.C + inputs.D;
    if (!isTheOtherThingThere(otherParam))
    {
        _log.Debug("Unable to do X because other thing isn't there.");
        return false;
    }

    var counter = StopWatch.StartNew();
    var flag = isWhatever(whatever);
    _log.Debug("Did whatever in {0} ms", counter.TotalMilliseconds);
    if (!flag)
    {
        _log.Debug("Unable to do X because whatever failed.");
        return false;
    }
    return true;
}

Then in your main code base:

if (CanDoSomething(some inputs))
{
    //Do the work
}

This eliminates the need for compound if logic, keeps the main program clean, and provides room to add the goo that tends to end up in production code.

Answer 10

While all these answers are good, nobody has discussed the long term maintainability of the code. The fact is most professional software engineers spend more of their time maintaining existing code bases then they do writing new code. We might not like it, but that's just a fact of the industry we are in.

So, if the code base is going to be used for a long period of time the more readable code is what you want because it's the easiest to maintain. But that maintenance effort is going to be a mixture adding new features and debugging. Or to put it another way, a well written bit of code should be easy to read AND easy to debug. If you say it the other way round this becomes obvious - hard to read code is going to be hard to debug!

Remember - the maintenance engineer might be you in 6 months time, after you've forgotten the details of your code. So save yourself the hassle and write clear to read code.

Personally, I find your second example easiest to read because those flags show me what is going on