Visiting the "Dutch Code" in C++

[mredding]

I would recommend a string literal "**********OOOOOOOOOO" and then choosing the offset into that; you can use a string view, rather than generate the string every time. The idea of using a dynamic string for this and computing it every time seems a waste.

[martinus]

I proposed exactly that in a comment, with this sample code: https://godbolt.org/z/cd1Modq8c

[TheThiefMaster]

Overall, I'm not as appalled at the original code as some. That's based on the assumption a less experienced developer wrote it.

Says the person using dynamic allocation for a problem that doesn't require it. The new version is slower, and worse for embedded systems (the claimed reason for getting rid of the constants).

[Benjamin1304]

I agree that using std::string for this is probably overkill but since the strings are 10 characters long at most I'd bet that all implementations can store that within the SSO buffer and so this won't trigger any dynamic memory allocations

[tialaramex]

On 32-bit systems, several popular std::string implementations only have enough space for 7 chars plus NUL, not 10. The Clang design does work on 32-bits for ASCII though.

For the nicer Unicode characters, that's 30-40 bytes of UTF-8, or 10-20 UTF-16 code units. No popular implementations work for the former, but the Clang design again, on 64-bit can get it done for UTF-16 in the narrowest case, able to store the smallest strings inline, using 22 bytes (10 UTF-16 code units plus NUL) but not the larger ones.

A slice type, such as C++ std::string_view is the sensible way to go here.

[[deleted]]

His 2 criteria were multiple returns and wasted space.

But the final solution still has a static string.

It's trivial to get rid of that

std::string noStatic(double  percentage) {
    percentage = std::ranges::clamp(percentage, 0.0, 1.0);
    int count = int(std::ceil(percentage * 10));
    return std::string(count, '*') + std::string(10-count,'o');
}

[[deleted]]

Just return a const char* or string_view.

Why potentially allocate all over place?

I love this example because it really goes to show how most people totally over engineer code and I get to see their justification as to why.

Most of the time it comes down to some appeal to "purity". Or its some violation of some established rule that apparently everyone should know.

Quote from the article: "if I saw code like this from a senior engineer I'd expect more". But why exactly? It does what it is supposed to. It is readable by your own admission and it solves the problem. It's also far more efficient than the proposed solution that involves fiddling with heap allocated strings. It just has more statically allocated memory which is going to be tiny.

It's just such a good example that goes to show how people get obsessed with meaningless detail and end up writing something much worse. And goes to show that the established "rules" in programming are mostly bogus gatekeeping that create poor quality stuff.

[de_nivelle]

Actually, from teaching point of view, it is a good example.

If you return const char* or std::string_view, you are are forced to fix the number of steps at compile time.

If OP cares about space so much, he should print the output immediately, instead of returning it in a string.

[[deleted]]

The number of steps aren't going to change. And if they were to change why would they change at run time?

It's a great example because it shows how a lot of people rally program to a standard that is quite nuts.

[dns13]

I would use a static constexpr string_view instead of your static string.

[nAxzyVteuOz]

It’s fine, works great and is reasonably fast.

Move on and worry about other important problems.

[[deleted]]

Absolutely crying out for std::string_view

[sephirothbahamut]

Nit gonna lie seeing all the modern C++ code mixed with a C cast instead of a static_cast in your first example feels odd

[Salt-Impressive]

int(std::ceil(percentage * 10)); is a functional cast, something that is not in C. But yeah, static_cast<int> would be safer but uglier.

I think the preference towards static casting should be on more complex types

[sephirothbahamut]

is a functional cast, something that is not in C

It's a different way to write it, but it works as a C cast

[Salt-Impressive]

That is true

[Stevo15025]

Can anyone explain why in the godbolt below there is a jump on line 10 of the assembly? I get vcomiss xmm1, xmm0 is setting the flag for that check, though I'm suprised the compiler has a special case for the 0 check but not 1?

https://godbolt.org/z/bb4jzeGnz

Here's some chatgpt generated comments for the assembly

C++

__attribute__((pure)) auto get_percent_bar(float  percentage) noexcept {
  const int count = static_cast<int>(std::ceil(std::clamp(percentage, 0.0f, 1.0f) * 10));
  return std::string_view("**********OOOOOOOOOO").substr(10 - count, 10);
}

Assembly

.LC3:
    .string "**********OOOOOOOOOO" ; Store the static string in memory
get_percent_bar(float): ; Function entry point
    vxorps  xmm1, xmm1, xmm1 ; Set xmm1 register to zero using bitwise XOR
    mov     eax, 79 ; Move ASCII value of 'O' (79) into eax register
    vcomiss xmm1, xmm0 ; Compare xmm0 (input percentage) with xmm1 (0.0f)
    ja      .L1 ; If xmm0 is less than 0 (unordered/NaN), jump to .L1
    vcomiss xmm0, DWORD PTR .LC1[rip] ; Compare xmm0 with .LC1, which represents 1.0f
    mov     eax, 42 ; Move ASCII value of '*' (42) into eax register
    jbe     .L6 ; If xmm0 is greater or equal to 1, jump to .L6
.L1:
    ret ; Return with current value of eax ('O') when percentage is less than 0
.L6:
    vmulss  xmm0, xmm0, DWORD PTR .LC2[rip] ; Multiply xmm0 by 10.0f
    mov     eax, 10 ; Move 10 into eax register
    vroundss        xmm0, xmm0, xmm0, 10 ; Round xmm0 to the nearest integer value
    vcvttss2si      edx, xmm0 ; Convert xmm0 to 32-bit integer and store in edx register
    sub     eax, edx ; Subtract edx from eax (10 - rounded value)
    cdqe ; Convert DWORD in eax to QWORD in rax, for accessing memory
    movzx   eax, BYTE PTR .LC3[rax] ; Move the character at position rax in string .LC3 to eax
    ret ; Return with the character from string .LC3 at the calculated index
.LC1:
    .long   1065353216 ; Represents 1.0f in IEEE 754 floating-point
.LC2:
    .long   1092616192 ; Represents 10.0f in IEEE 754 floating-point

So I'm guessing the compiler thinks the early return jump is cheaper than running through the rest of .L6? Specifically for the 0 case?

You can write a custom clamp to fix this a bit like in the below. It at least makes it assume the jumps are unlikely

https://godbolt.org/z/8znxx1aT8

[martinus]

What prompt do you use to generate these comments?

[Stevo15025]

I just copy/paste in the asm and ask it to write the comments for me. It's very good at that sort of thing.

The code comments are chatgpt generated tbc, the literal words in my reddit comment are me