Steven Schveighoffer Wrote: > On Tue, 24 Nov 2009 16:52:52 -0500, Bartosz Milewski > <bartosz-nos...@relisoft.com> wrote: > > > Steven Schveighoffer Wrote: > >> > Imagine you're reviewing this code written by a relative newbee: > >> > > >> > char[] quote(char[] word) { > >> > word.length += 2; > >> > word[length-1] = '"'; > >> > for(int i = word.length-2; i > 0; --i) > >> > word[i] = word[i - 1]; > >> > word[0] = '"'; > >> > return word; > >> > } > >> > > >> > void storeFoos(char[] buffer, string pattern, Container c) > >> > { > >> > char[] res = find(buffer, pattern); > >> > c.store(quote(res[0..pattern.len]); > >> > } > >> > > >> > Is this buggy? If so, how and when will the bug manifest itself? > >> > >> In my mind it is not buggy. If quote or storeFoos is not meant to be > >> able > >> to molest the data, it should require a const(char)[]. If this was D1 > >> code, I'd say it depends on the purpose of storeFoos, if storeFoos is > >> expecting to own the input buffer, it's not buggy. If it's not, then > >> storeFoos is buggy (in all cases). But we aren't talking about D1. > > Yes, a lot depends on what's expected of storeFoos. Suppose that the > > surrounding code expects the buffer not to be modified. The newbee > > didn't want to idup the buffer because in most buffers the pattern is > > not found. > > Then he forgot the const decoration for the parameter. No duping is > necessary: > > void storeFoos(const(char)[] buffer, string pattern, Container c) > { > ... // will result in compiler errors for given quote function, fix > that. > }
Well, he tried that, but then his quote function wouldn't work and he needs it in other places as well. You know how they are ;-). Besides he thinks "const" is for wimps. He won't stop arguing until you prove to him that there is an actual bug in his code. > > > >> > Or another story. An enthusiastic programmer comes to you with a very > >> > clever rewrite of a function. This is the original: > >> > > >> > T[] duplicate(T)(T[] src) { > >> > static if (is(T == invariant)) > >> > return src.idup; > >> > else > >> > return src.dup; > >> > } > >> > > >> > This is his improved version: > >> > > >> > T[] duplicate(T)(T[] src) { > >> > T tmp = src[$-1]; > >> > src.length -= 1; > >> > src ~= tmp; > >> > return src; > >> > } > >> > > >> > No need for static if! The extension is bound to re-allocate because > >> of > >> > stomping rules. Will this always work, or is there a gotcha? > >> > >> He forgot to check for length 0. > >> > > > > That he did. So now he goes back to the drawing board and adds an early > > return for length 0. Is his code correct now? > > Yes. > > I have a feeling you have some trick up your sleeve that you think makes > it incorrect :) Well, I was wondering if it could be rewritten as: T[] duplicate(T)(T[] src) { assert(src.length != 0); T tmp = src[$-1]; src.length -= 1; src.length += 1; src[$-1] = tmp; return src; } and whether it's okay for the optimizer to do some simple arithmetic optimization.