Switch to uint8_t, some optimizations

[davepl]

Description

Replace this text with your description.

Contributing requirements

• I read the contribution guidelines in CONTRIBUTING.md.
• I placed my solution in the correct solution folder.
• I added a README.md with the right badge(s).
• I added a Dockerfile that builds and runs my solution.
• I selected drag-race as the target branch.
• All code herein is licensed compatible with BSD-3.

[rbergen]

I always like improvements to one of the original solutions! :)

I have two comments, the second one (array initialization) is the one I think actually matters.

[rbergen]

Is it necessary to declare arrSize as a member variable? It's private so only visible to the BitArray member functions, and I don't think it's used anywhere outside the constructor.

[rbergen]

Are you absolutely sure? When I was learning C++, it was hammered into me by my teacher that with C and C++, one has to assume garbage when allocating memory via new.

I actually looked up the documentation in the C++ reference, and this is what it says about default initialization, which is what's done when new T is used ("if T is an array type, every element of the array is default-initialized", as per Default-initialization):

When storage for an object with automatic or dynamic storage duration is obtained, the object has an indeterminate value.

If no initialization is performed for an object, that object retains an indeterminate value until that value is replaced.

It may work by accident in your office, but it looks to me we can't rely on that happening elsewhere. :)

[davepl]

Fixed!

…

On Oct 20, 2024, at 12:22 PM, Rutger van Bergen ***@***.***> wrote: @rbergen commented on this pull request. I always like improvements to one of the original solutions! :) I have two comments, the second one (array initialization) is the one I think actually matters. In PrimeCPP/solution_2/PrimeCPP_PAR.cpp <#979 (comment)>: > @@ -24,80 +24,66 @@ using namespace std::chrono; const uint64_t DEFAULT_UPPER_LIMIT = 10'000'000LLU; class BitArray { - uint32_t *array; + uint8_t *array; size_t arrSize; Is it necessary to declare arrSize as a member variable? It's private so only visible to the BitArray member functions, and I don't think it's used anywhere outside the constructor. In PrimeCPP/solution_2/PrimeCPP_PAR.cpp <#979 (comment)>: > + // Bits are left at zero default, so no need to initialize them + // std::memset(array, 0x00, arraySize(arrSize)); Are you absolutely sure? When I was learning C++, it was hammered into me by my teacher that with C and C++, one has to assume garbage when allocating memory via new. I actually looked up the documentation in the C++ reference, and this is what it says about default initialization, which is what's done when new T is used ("if T is an array type, every element of the array is default-initialized", as per Default-initialization <https://en.cppreference.com/w/cpp/language/default_initialization>): When storage for an object with automatic or dynamic storage duration is obtained, the object has an indeterminate value. If no initialization is performed for an object, that object retains an indeterminate value until that value is replaced. It may work by accident in your office, but it looks to me we can't rely on that happening elsewhere. :) — Reply to this email directly, view it on GitHub <#979 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCF4HHMVWOYHM4TLP3NDZ4P7GNAVCNFSM6AAAAABQHWDEHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBQGI2TIOJZGU>. You are receiving this because you authored the thread.

[rbergen]

Thanks, LGTM!