[Webkit-unassigned] [Bug 22479] Consider renaming to 'deepCopy' the 'copy' methods that perform a deep copy

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22479





------- Comment #6 from ggaren at apple.com  2008-11-25 17:13 PDT -------
> Maybe I'm just confused trying to understand why copy() is bad - shallow copy
> is the default one, so a method named copy() should be expected to do something
> different, shouldn't it?

There's disagreement about what the default is. Anders thought that deep copy
was the default. Turns out, neither is the default. WebCore is split about 50 /
50, and nothing is the default.

I think it's important to specify the type of copy in the name because, in a
world where there is no default, "copy" is ambiguous. The proof is in the
pudding: Each use of copy is annotated by a comment that says "this is a deep
copy." That's bad programming 101. If "copy" means "deepCopy", and needs a
comment to say so, we should just rename it to "deepCopy," and get rid of the
comment.

I think StringImpl::copy is indeed a "deep" copy. Based on your comments, it
seems like it might be invalid to try to optimize copy by not copying
ThreadSafeShared data, so I think it is the case that all instances of "copy"
that claim to perform a "deep" copy do so, and need to do so.

> The best I can think of is "copyOut()" - it's somewhat mysterious, but concise,
> and close to the meaning that it has in kernel programming. But we won't have a
> matching "copyIn()". Or maybe we could just keep "copy()".

You're oversimplifying. If we do nothing, we keep a function named "copy()"
where each use is annotated by a comment that says "this is a deep copy." That
seems like the worst of all possible worlds.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.