Timeline for Computing the first n primes: an example from Clean Code revised for C++
Current License: CC BY-SA 4.0
9 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Aug 23, 2019 at 14:52 | comment | added | L. F. | @SG_90 About short functions: everything good becomes bad when used to the extreme. In general, short and nice functions are great, but too many of them just feels a bit wrong IMO. In this case, you are logically doing one thing in this function, and things like cross tables aren’t a logically separate matter I think. I am glad to see my answer is being read so carefully! | |
| Aug 23, 2019 at 14:50 | comment | added | L. F. |
@SG_90 There is nothing wrong with raw loops whatsoever. What’s wrong is loops that really aren’t logically loops (e.g., zeroing a range). std::size_t is used when you are dealing with size, e.g., as the size of an array. The fast family of types are not directly related to speed ... There is no objective way to judge which type is “fast,” and I just consider them as the “natural” type to use. The alternative is the least family, which may actually incur performance overhead. And std::sqrt may compute the square root of large numbers wrongly, and may be inefficient.
|
|
| Aug 23, 2019 at 9:43 | comment | added | SG_90 |
I also have one comment to your own solution. Naming a vector or an array a 'table' doesn't really describe what it's for. crossedOut, which is not my idea, accomplishes this far better. Cheers!
|
|
| Aug 23, 2019 at 9:43 | comment | added | SG_90 |
Ad. std::uint_fast32_t. I didn't know about these types. Cool, thanks. Good for when you care about maximally optimising your algorithms for speed. Ad. isqrt. I'm wondering if this is really necessary. I might be missing something, though, so could you clarify at what point do you see this becoming a problem? Overall, thanks for the comments, I learned something. If I may give you some feedback, then I missed some arguments justifying your suggestions, such as with writing short functions.
|
|
| Aug 23, 2019 at 9:40 | comment | added | SG_90 |
Ad. std::vector<bool>. I had a vague idea there was something wrong with it, but didn't know exactly what. Thanks! Ad. size_t vs std::size_t. Oh right, size_t actually comes from the C compatibility header. True, we should prefer the namespaced one, though they're essentially the same, I think. std::size_t is not as convenient, however ;) Still, you're right. Ad. using number_t. That's an interesting idea, thanks. Where would you use size_t, then?
|
|
| Aug 23, 2019 at 9:39 | comment | added | SG_90 |
Thanks for the valuable input! There are points I agree with, and some I disagree with, as in life :) Ad. Small functions. I do like small functions, and I don't think "it's just weird" is enough of a justification not to write them. Another thing I don't like are raw loops (S. Parent's talk on this is highly recommendable: youtu.be/W2tWOdzgXHA), so I'd insist on keeping crossOutMultiples along with putUncrossedIntegersIntoResult (but rename it). I do think you've got a point, though, and I'd include crossOutMultiplesOf in the former plus get rid of determineIterationLimit.
|
|
| Aug 21, 2019 at 9:27 | history | edited | L. F. | CC BY-SA 4.0 |
added 86 characters in body
|
| Aug 21, 2019 at 9:18 | history | edited | L. F. | CC BY-SA 4.0 |
added 1021 characters in body
|
| Aug 21, 2019 at 9:11 | history | answered | L. F. | CC BY-SA 4.0 |