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.
}


> 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 :)

-Steve

Reply via email to