Timeline for Making multiple copies of a pattern
Current License: CC BY-SA 3.0
16 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Dec 18, 2014 at 18:55 | vote | accept | CommunityBot | ||
| Dec 17, 2014 at 19:42 | comment | added | user59064 | PS: Upvoted all your bits. I wanted to upvote your edits but I don't think that's a thing. I might not agree with all your points but I think we only really disagree on matters of style. | |
| Dec 17, 2014 at 19:40 | comment | added | user59064 |
I unit tested all combinations of pattern from 1 - 2048 chars and targets from 1 - 4096 chars to make sure I got all the boundary conditions such as power of two, one over, one under, on the money, etc. Very similar to you I wrote a 'verify' function and filled the target before and used rand() generated patterns but using a fixed seed (for reproducibility).
|
|
| Dec 17, 2014 at 19:38 | comment | added | user59064 | You're performance tests are consistent with what I saw. The gains aren't great (definitely not even *1/2) but they are better for bigger buffers as you might expect. | |
| Dec 17, 2014 at 17:53 | comment | added | user59064 |
If no one does it for me... I'll be putting the missing const and that 'single copy' shortcut in the original post when I've had chance to test them. I'm sure they're right but as my earlier goof shows, it's easy to mess code up by copy editing.
|
|
| Dec 17, 2014 at 17:52 | comment | added | user59064 | As it happens I have a little test harness and performance test up my sleeve and as mentioned compared it to a simpler copy * n. I omitted them from the post as taking up too much space. I wish there was markup to post and fold away such extras. | |
| Dec 17, 2014 at 17:40 | history | edited | Edward | CC BY-SA 3.0 |
added test driver and results
|
| Dec 17, 2014 at 16:25 | comment | added | user59064 | I think the whitespace problem is even worse on this forum. The width of the available space is tiny! As I say a matter of taste. I accept my style is slightly unusual. I've drifted around between C, VB, Objective-C, Java, Transact-SQL and that I've picked up all sorts of habits. I find the 'l' and 'p' prefixes (and 'm' or 'f' for members/fields) useful. I absolutely don't find any other name decorations (pre- or post-) helpful at all. | |
| Dec 17, 2014 at 16:21 | comment | added | user59064 |
I think modifying parameters is dreadful style and causes a whole class of bugs in maintenance. Equally I think variables should be declared 'const' whenever they can. Again a whole class of bugs are captured in maintenance. There's no real reason to hold back! NB: You haven't called me out on the real const error that I didn't declare pFillWith as const void. That's an outright problem (with my code). You've fixed it but not made the call.
|
|
| Dec 17, 2014 at 16:19 | comment | added | Edward | If you're the only one who is ever going to read or use the code, then the conventions you use only need to please yourself. I write a lot of code like that. However, if you might be sharing the code with other people, things like whitespace tend to matter more. Ultimately, as the author, you're the arbiter of what should and should not be changed. CodeReview is about suggestions that might improve your code, but you are, of course, free to adopt what you like and leave the rest. | |
| Dec 17, 2014 at 16:15 | comment | added | user59064 | I think naming is a matter of taste and don't like whitespace because it's all too easily end up splitting lines or making them scroll off to the right. | |
| Dec 17, 2014 at 16:14 | comment | added | user59064 |
I agree in principle about combining the conditions. However I also agree about returning an error-code. But there not being so much of a convention (cf: memcpy) on returning error-codes for invalid arguments I actually left the if statements separate as a 'hook' for error reporting. Notice the first if statement is 'nothing to do' and the second is 'invalid argument'.
|
|
| Dec 17, 2014 at 16:12 | comment | added | user59064 |
I'll reply to all these points in separate comments. Good spot on if(pToFillSize<=pFillWithSize){. When the target is the same size as the source we can still make the early exit!
|
|
| Dec 17, 2014 at 15:29 | history | edited | Edward | CC BY-SA 3.0 |
fixed typo
|
| Dec 17, 2014 at 15:12 | vote | accept | CommunityBot | ||
| Dec 17, 2014 at 15:12 | |||||
| Dec 17, 2014 at 15:09 | history | answered | Edward | CC BY-SA 3.0 |