Also available in Italian

On Twitter recently someone posted this picture:

Ok, that’s C#, but we can (and will, in a few lines) rewrite it in C++.

In the twitter responses, many people made fun of the code, or complained that the function is full of magic numbers, some others that it’s too long (implying that the cost of maintenance is proportional to the number of lines). And I was tempted to do the same, but something stopped me.

I realized nobody asked “What the problem was?”.

Let’s assume, for a moment, that the problem was exactly that – filling in precisely 10 balls, a number of balls proportional to the percentage. Is this a terrible solution?

So I decided, this is going to be today’s exercise – “wasting” an entire evening on a progress bar made of 10 balls.

The code

This is a C++ blog, so the first step will be rewriting the code in C++. I made it just a bit better, removing half of the constants… but I hope you’ll agree that, in spirit, it’s not too far off from the original.

const char * ProgressBugFixedSimplified(double percent)
{
    if (percent == 0.0) return "..........";
    if (percent <= 0.1) return "o.........";
    if (percent <= 0.2) return "oo........";
    if (percent <= 0.3) return "ooo.......";
    if (percent <= 0.4) return "oooo......";
    if (percent <= 0.5) return "ooooo.....";
    if (percent <= 0.6) return "oooooo....";
    if (percent <= 0.7) return "ooooooo...";
    if (percent <= 0.8) return "oooooooo..";
    if (percent <= 0.9) return "ooooooooo.";
    return "oooooooooo";
}

Does it solve the problem? Yes – almost.

The bugfix

My only contribution to the thread ended up being just this: fixing what I consider to be a bug.

For negative numbers, it returns a full bar, while in my opinion the bar should be empty. In some cases, indeed, numerical approximations can produce a slightly negative values when the numbers go around 0, and thus we could see a full bar when just at the beginning of our progress. It’s an easy fix: we just need to change the first test to be <=, and we’re done.

const char * ProgressBugFixedSimplified(double percent)
{
    if (percent <= 0.0) return "..........";
    if (percent <= 0.1) return "o.........";
    if (percent <= 0.2) return "oo........";
    if (percent <= 0.3) return "ooo.......";
    if (percent <= 0.4) return "oooo......";
    if (percent <= 0.5) return "ooooo.....";
    if (percent <= 0.6) return "oooooo....";
    if (percent <= 0.7) return "ooooooo...";
    if (percent <= 0.8) return "oooooooo..";
    if (percent <= 0.9) return "ooooooooo.";
    return "oooooooooo";
}

For the sake of this analysis, I took a few metrics: the code and the data size, number of calls and the presence of exceptions. In all cases, I used GCC 12.2, and the settings -O1 -std=c++20.

So, for this code, the results are:

Code size: 206 bytes
Data size: 121 bytes
Calls: none
Exceptions: no

Is there a shorter/better/smarter way to solve the problem? Hell yeah, we’re engineers, we can engineer the hell out of this function. Let’s start.

The loop

Ok, we need to collect enough balls to create our string, and fill the rest with dots. For simplicity, we’ll use std::string, just because we can.

std::string ProgressLoop(double percent)
{
    const int elements = 10;
    double threshold = percent * elements;
    std::string result = "";
    for (int i = 0; i < elements; ++i)
    {
        result += (i < threshold) ? "o" : ".";
    }
    return result;
}

Better?

I can change the number of balls, sure… but remember, that wasn’t in the requirements.

The code begins to be less readable, but we can still understand what’s happening.

Code size: 425 bytes (without external code)
Data size: 0 bytes
Calls: 8 – throw (3x), delete (2x), new (1x), unwind (1x), memcpy (1x)
Exception: yes

What a fail! the footprint is bigger, it allocates, deallocates, and copy memory, and it can even throw exceptions in three different points.

To be completely honest, we won’t probably incur in any of those calls, because this one will be well within the limits of SSO – Small String Optimization (see the nice blog post from Joel Laity libc++’s implementation of std::string for details). But still, the code is there.

Passing memory around?

We could avoid std::string and return an array of characters? No, that’s verboten.

Returning a pointer to char? Someone would need to allocate that, or receive the pointer via parameter, the interface would be more complex and error prone.

So, to cut it short… let’s see who…

…the winner is…

Here it is, my best non-allocating, string-like-returning, non-throwing, small-footprint version!

std::string_view Progress(double percent)
{
    constexpr char bar[] = "oooooooooo........";
    constexpr int elements = std::size(bar)/2;
    double threshold = percent * elements;
    return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}

Nice, isn’t it?

The code is terribly small, the data is smaller than before, it just do some “simple” calculations, and solves the problem elegantly.

Except it’s wrong, as it goes out-of-bound every time you give a negative value, or a number above 1, crashing miserably.

But we can fix it, right? Sure…

std::string_view Progress(double percent)
{
    constexpr char bar[] = "oooooooooo.........";
    constexpr int elements = 10;
    double threshold = std::clamp<double>(percent * elements, 0, elements);
    return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}

You love it already. I can feel it.

It’s just 4 lines, and one two are just declarations of constants, how hard it can be to maintain, right?

Right?

Wrong.

The code has a bug, and you didn’t even noticed it. The correct code is this:

std::string_view Progress(double percent)
{
    constexpr char bar[] = "oooooooooo..........";
    constexpr int elements = 10;
    double threshold = std::clamp<double>(percent * elements, 0, elements);
    return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}

You can spot the difference, right? Well, the bug was not on one of the two uber-complex lines at the bottom, it was in the very first line. A missing dot. That would have caused a nice out-of-buffer read.

So, not even the “easy” part of this code is that easy to read or maintain, after all. And who can read the two bottom lines? Why the template parameter on clamp? Why the double doesn’t narrows to int in the array accessors? (no answer to these questions today, this is a simple blog post)

But let’s give the metrics anyway:

Code: 87
Data: 21
Calls: no
Exceptions: no

Good! Much better! We win!

Is it a victory?

The experience

I wanted to call this section “the verdict”, but I don’t feel like a judge today – I actually feel a bit stupid* for having initially laughed of the code, without realizing it was a simple and relatively elegant way to reach the goal.

But this made me think, and ask myself questions:

Can the original code be maintained by a junior developer (say, a new college graduate)? Yes.
What about my final code? Probably not. So we would need to pay a senior dev to maintain a progress bar.

Can the original code be written easily without a bug? Yes. And if bug happens, they’re easy to spot.
What about my final code? Not by me. To be clear, both bug on this post were intentional, but I did introduce other bugs while writing the intermediate code that didn’t appear in this code – if you’re lucky, you can still watch the entire 1h rant on twitch, and see my buggy code.

In the end, what is the cost of maintaining the original code? Probably small, it’s intuitive and easy to read.
What about my final code? In my opinion, terribly high. If I had to read that code 1 week from now, I’ll probably have to spend a minute or two to check if the call to clamp and the creation of string_view are correct.

And finally, the most important question of all was: would I, an engineer with 25+ years experience in C++, enjoy maintaining a progress bar function, just because nobody else in my team could efficiently do that?

No.

I might not change my coding style tomorrow, but this exercise definitely changed the way I’ll see the “maintenance cost”, and I’ll surely think about these aspects more often from now on.

* I’m always happy when I feel stupid, because it means I’ve learnt something.

Update 2023-01-23: The version I wrote was still buggy. Two of you (Vinzenz in the comments, and @emilianobr on Twitter) reported that the final code was buggy, and indeed I incorrectly assumed static was implied there (I seem to recall it worked on MSVC during my tests, but then failed on Clang and GCC when tested on godbolt). I’m not sure why Clang and GCC wouldn’t use a global constant for that (I need to re-read the standard about that, but nonetheless I’m pretty sure it’s my bug!). I feel this makes the entire content of this post even more valid.
Anyway, here’s the fixed code:

std::string_view Progress(double percent)
{
    static constexpr char bar[] = "oooooooooo..........";
    constexpr int elements = 10;
    double threshold = std::clamp<double>(percent * elements, 0, elements);
    return { &bar[int(elements - threshold)], &bar[int(2 * elements - threshold)] };
}

At the same time I fixed another couple of small mistakes

Update 2023-01-23 10:00 UTC: Fixed typos, and bugs around.

• Missing fix in “fixed” version (reported by Fabian Sandin in the comments)
• Typo in text (reported via a Tweet that has been then deleted).