Copy semantics of resources

[Nexus]

Hi, a recent bug of mine led me to think about the resource classes in SFML. I intended to write something like this:

Code: [Select]

const sf::Image& Image = Blabla.GetImage(3);
sf::Sprite Sprite(Image);
Unfortunately, I forgot the reference.

Code: [Select]

const sf::Image Image = Blabla.GetImage(3);
sf::Sprite Sprite(Image);
You can imagine the massive loss of performance. That confused me a little, I thought SFML's big resource classes like sf::Image and sf::SoundBuffer wouldn't support copy constructors. Were those already there in older versions? Or did the resource classes once inherit from sf::NonCopyable? By the way, the same applies to assignment operators.

In my opinion, the copy constructor at such huge classes is a source of error which shouldn't be underestimated. Implicit conversions happen so fast in C++, for example in the context of function parameters. I agree, experienced C++ programmers must be aware of that (nevertheless, I had to debug several minutes).

But the important point is, I don't see a real use case of the copy constructors. Resources are rarely completely copied. In the few cases they are, functions like sf::Image::Copy() should be sufficient - and are much safer. Wouldn't deriving the SFML resource classes from sf::NonCopyable be a suitable option?

[Laurent]

Resource classes have always been copyable in SFML.

A simple use case for copy is storage in standard containers. Without copy you would have to store pointers, which means finding a suitable smart pointer class in an external library (std::auto_ptr is not compatible with containers).

I personnally make a class non-copyable when it really is, I mean when making it copyable would lead to errors or situations that are impossible to handle. But clearly not if the class is just heavy to copy.

[Nexus]

A simple use case for copy is storage in standard containers.

Using huge classes in standard containers is mostly nothing more than a waste of performance. Especially, a copy as functionality of duplicating values is not intended, but must be done as a result of the container's requirements. That doesn't seem like a good use case for me. And to be honest: people, that just want to store (not copy) resources and that are reliant on dynamic container operations, choose (smart) pointers or other containers - at least if they have got some C++ know how. I cannot imagine many situations where a copy itsself is wanted.

Without copy you would have to store pointers, which means finding a suitable smart pointer class in an external library (std::auto_ptr is not compatible with containers).

A lot of people are using Boost. Additionally, shared_ptr is in the TR1. Others can still use new and delete. If they write functions for encapsulation, the memory management is not even error-prone. Allowing resource-copies to relieve users from handling pointers and thinking about efficiency can't be the decisive point?

I personnally make a class non-copyable when it really is, I mean when making it copyable would lead to errors or situations that are impossible to handle. But clearly not if the class is just heavy to copy.

Okay, that's an argument I can rather reconstruct; I guess that is one of your personal design decisions. I just think a class shouldn't have a functionality which is itsself hardly needed, but makes life easier because one doesn't have to care about performance and pointers any more.

[Laurent]

In the real life, I hardly need performance when I manipulate a container of big objects. So I'm rather happy to find these big objects copyable, and to be able to store them directly. If I want it to be fast, I can still use pointers. I'm free to use whatever I like.

I don't think limiting the usage of resource classes would make everyone happy.

[Imbue]

If you don't want to copy the images, uh, then don't copy them. Having a copy constructor is useful for the people who need it. (I've used it to copy and then modify an image, for example.)

Perhaps, to fix your bug and prevent it from happening again, you should instead be using plan old pointers. For my game, I store pointers for all loaded images in one map. Then, when I need an image, I just grab the pointer from the map. It's quite difficult to accidentally copy an Image from a pointer. Smart pointers aren't needed, as long as you understand only one part of your code is responsible for cleaning up.

[Nexus]

In the real life, I hardly need performance when I manipulate a container of big objects.

I suppose you mean the situations in which big objects are copied are mostly not performance-critical (for example at initialization time). Anyway, that's still no reason to waste.
(No, I understand you wanting to be user-friendly, don't worry ).

So I'm rather happy to find these big objects copyable, and to be able to store them directly. If I want it to be fast, I can still use pointers. I'm free to use whatever I like.

Well, providing different ways makes more users happy, I agree.
I only considered the ability to copy rather a risk than a help, but quite possibly I was mistaken. I assumed this ability would be "abused"  in most cases (for actually useless copies) - which seems to me like a reason not to make this functionality available.

If you don't want to copy the images, uh, then don't copy them. Having a copy constructor is useful for the people who need it. (I've used it to copy and then modify an image, for example.)

Okay, I didn't know there would be several use cases for "real" copies. Could some other users comment that? I never know what the rest of the community thinks about a topic. :wink:

Perhaps, to fix your bug and prevent it from happening again, you should instead be using plan old pointers.

No, certainly not. References have their advantages. I also use pointers when they're required, but not everywhere. By the way, the bug was just the reason why I noticed the resource copy semantics. I believe it won't happen that soon again...

[Tank]

I only considered the ability to copy rather a risk than a help, but quite possibly I was mistaken. I assumed this ability would be "abused"  in most cases (for actually useless copies) - which seems to me like a reason not to make this functionality available.

IMHO it's only "abused" when the developer doesn't know what he's doing. I think SFML shouldn't provide mechanisms to help developers making less errors -- this is the task of each developer.

Okay, I didn't know there would be several use cases for "real" copies. Could some other users comment that? I never know what the rest of the community thinks about a topic. :wink:

It's not really the question of if it's needed or not, but IF it's needed, you can do it.

By the way, the bug was just the reason why I noticed the resource copy semantics. I believe it won't happen that soon again...

And that's it: it was a *bug*. If someone needs to really copy an image (because for modifying a copy or whatever), it won't be a bug.

Writing correct code makes this discussion useless.

[Nexus]

Writing correct code makes this discussion useless.

No - that's what I wanted to express with my "by the way". The bug was just the trigger that made me notice the copy semantics. I didn't know about them before. And I didn't start a discussion about this topic because I was so scared of my bug happening again.
As it happened at me and I am aware of the semantics now, it probably won't happen too often in future. But I thought there might be other users with a similar concern.

However, I inteded to question the copy ctor and assignment operator of resources in a general way.

IMHO it's only "abused" when the developer doesn't know what he's doing. I think SFML shouldn't provide mechanisms to help developers making less errors -- this is the task of each developer.

The point was to discuss if the semantics are really needed by SFML users. In my opinion, features that carry a risk are worth discussing, especially if the advantage of an existing feature is doubted (as it was at least by me). But I am repeating myself...

And yes, SFML should at least try to minimize error sources which result of wrong library design. I don't say the status quo is wrong, it's meant as general statement. Minimizing error sources: This may one of the reasons why SFML is written in modern, abstracted C++ and makes use of a lot of efficient, typesafe and few low-level programming techniques.

It's not really the question of if it's needed or not, but IF it's needed, you can do it.

For the most part, you are right. But this argument could be applied to most features. A sf::Sound member function to rewind? It's not really needed, but IF, you can do it.

I understand that there are some real cases for copies. Unfortunately, I am trying to explain my motives again and again. Apart of some exceptions, they seem to be either ignored or misunderstood. I hope some people can at least reconstruct them - even if they aren't sharing my opinion.

To clarify it, I do understand what was the idea behind copyable resources. It would just have been nice if there were a little bit more descriptive examples.

[Laurent]

I understand your point of view Nexus, and actually I agree with it. I personnally prefer clean, robust and safe design over a bloated interface that does almost everything. That's why I refuse a lot of requests that are easy to implement

But in this case, enabling copy is consistent with the class semantics, I don't really like the reason why you want to disable it.

[Nexus]

But in this case, enabling copy is consistent with the class semantics, I don't really like the reason why you want to disable it.

Okay, I can accept this.

I hope you don't mind if I question some design decisions in SFML, even if they are really justified. Sometimes I am just curious. Anyway, thanks for the invested time!

[Laurent]

I hope you don't mind if I question some design decisions in SFML

Absolutely not, I love discussing about design choices. That can only make SFML better

[Tank]

I do also fully understand what you're complaining about, Nexus. But I share Laurent's opinion that it's absolutely consistent with the whole design. Also, I dislike the idea of disabling certain features just to get the library a bit more foolproof.

I guess you would be happier with disabling the copy constructor (and assignment operator) and implement something like Clone(), right? I would like that too, but since the whole library doesn't follow this approach, I'm absolutely fine with the current design.

Just my 2ct.