21

Surprisingly, String.Clone() doesn't return a copy of a string as String.Copy() would do. Instead, it returns 'this', the original string.

I would like to understand why the .Net Framework team choose to go this way.

As per MSDN:

The ICloneable interface [...] requires that your implementation of the Clone method return a copy of the current object instance.

String.Clone() clearly doesn't follow this guideline.

I know that strings are immutable, but if immutability was the reason here, String.Copy() would also return this but it doesn't.

This is a rather theoretical question, of course.

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Sylvain Rodrigue
  • 4,751
  • 5
  • 53
  • 67
  • 3
    As strings are immutable as well as interred, the is no difference between string.clone and string.copy as they relate to your question. – Sam Axe Dec 18 '13 at 20:11
  • Firstly, `ICloneable` is a notoriously bad interface to depend upon. Is it shallow clone? Is it deep clone? Is it just a reference to the same immutable object for performance reasons? Not to mention boxing of value types. Secondly, if I had to hazard a guess as to why the team did this, it would be for performance and reduced memory usage. But overall, it's just a guess; not sure if it's a good question for SO. – Chris Sinclair Dec 18 '13 at 20:11
  • 4
    This question appears to be off-topic because it is not a *practical* problem about programming, but rather an academic one. – Servy Dec 18 '13 at 20:12
  • @Dan-o All strings are not interned. Compile time literal strings are interned only so long as that compiler option is enabled, and strings created at runtime are interned only if the programmer explicitly interns them. – Servy Dec 18 '13 at 20:13
  • I agree. From my perspective, there is no difference. – Sylvain Rodrigue Dec 18 '13 at 20:14
  • 4
    This can easily be turned into a *practical* problem by asking "How should I implement `ICloneable` on my custom immutable type?" –  Dec 18 '13 at 20:15
  • 1
    @hvd That would make it primarily opinion based, and also likely require discussing specifics; is there a compelling reason you need a deep copy, is a shallow copy sufficient for your purposes, what is the reason you're implementing `ICloneable` in the first place? – Servy Dec 18 '13 at 20:15
  • Also see [this question](http://stackoverflow.com/questions/3465377/whats-the-use-of-string-clone). – Jakob Möllås Dec 18 '13 at 20:16
  • @ChrisSinclair : yep, I never use ICloneable. And maybe it is not a good question for most of SO users - I just try to understand why things are done the way they are. And I learn a lot doing so :) – Sylvain Rodrigue Dec 18 '13 at 20:16
  • @Servy Either returning `this` is according to spec, or it isn't. There's no opinion, only different interpretations of facts. Your edited comment has some good points though, and I agree that it might not be a useful question without more info. –  Dec 18 '13 at 20:17
  • 2
    @hvd Whether or not the spec should define it as a deep/shallow copy *is* an opinion, which is what you were proposing changing the question to. This question, as it stands, is not opinion based, but rather offtopic for entirely different reasons. – Servy Dec 18 '13 at 20:18
  • 1
    It returns *this* because the original programmer that returned a copy got fired for incompetence. – Hans Passant Dec 18 '13 at 20:21
  • 1
    @Servy That's neither what I was trying to say, nor what I did say, but it is very nearly what I did say, so I can see the confusion. Regardless, this is no longer a productive discussion, so I'll drop it. –  Dec 18 '13 at 20:21
  • Guys, check out my answer. Don't think that it is downvoted and therefore worthless. I've pasted the actual implementation of the clone and copy from Microsoft's website – Amit Joki Apr 13 '14 at 09:10

3 Answers3

5

How could you detect the difference? Only by comparing the two references using object.ReferenceEquals. But by any semantic operation on the string you can't tell the difference.

Comparing strings by reference is almost always a bug to begin with because you can rarely rely on interning to happen or not happen.

This issue does not only apply to String. If you had an immutable Point class, why would you return a fresh object from Clone? No need.

IClonable is rarely used and rarely useful, anyway. If you want to expose users of your class a way to obtain a copy of a given instance you don't need to inherit from IClonable at all.

usr
  • 168,620
  • 35
  • 240
  • 369
2

IClonable is somewhat deprecated as it's unclear what "Clone" means from a system-wide standpoint (deep, shallow...). See http://blogs.msdn.com/b/brada/archive/2003/04/09/49935.aspx

The reference source documents the Clone method with the following comment:

// There's no point in cloning a string since they're immutable, so we simply return this.

Interning of strings means that it's hard to collect strings (they can be referenced more than once) which means really making a new copy of string serves only to stress the system. Plus, interning and copying conflict with each other--so the general rule of interning wins.

Peter Ritchie
  • 35,463
  • 9
  • 80
  • 98
1

As has been mentioned, since strings are read-only, Clone() behaves reasonably. You virtually never actually need two separate instances of the string, and by not making a copy, memory is saved. In the very rare case that you actually need a copy (for some reason you want Object.ReferenceEquals to return false), you can use String.Copy() instead.

It may seem pointless to have a method that just returns this. The reason to have such a method is to implement ICloneable, and I agree that String should implement ICloneable so that generic code like 

T Foo<T>(T x, ...) where T:ICloneable {/* code that might clone x*/}

can be compatible with String.

It's a little strange to me that the method is public, though, since there's no reason to call it directly. It would make sense if it were only accessible through a reference to ICloneable.

Qwertie
  • 16,354
  • 20
  • 105
  • 148