Unloadable Resources

[Nexus]

Code: [Select]

bool Load( const std::string& filename, sf::Image& image );

Hm, that's not what I understood by "static create method"

I don't see the advantage of this function over the current

Code: [Select]

sf::Image;
Image.LoadFromFile("image.bmp");
A public default constructor is still necessary (or what do you pass to the sf::Image& parameter?), you cannot prevent empty objects, besides the signature is unintuitive because the (quasi) this object is not the first parameter.

[Fred_FS]

I wouldn't call them "uninitialized" or "invalid", but "empty".

Then sf::Image::Unload() or sf::Image::Clear() would just be fine. To be honest I can't really think of situations where it's really needed, but for consistency I'd say it's okay.

I don't like the word "consistency", because if a function is not needed you should not implement it in my opinion. The very simple reason is, that it is easy to use it in a wrong way. You would be able to clear an image, but you also would be able, to "use" this empty image again. And so of course an image can be empty, but it isn't supposed to.

I rather would prefer not to offer any default constructor. So that the user had to initialise the image immediately after creating it.
If you cant initialise it immediately(see Laurent's example) I would use pointers(I would use them even with a default constructor, because it is very simple to check, weather a pointer is NULL or not and so you can prevent missuse of an uninitialised image).
So the code would look like:

Code: [Select]


class Game
{
public:
    Game()
        : tileset()
    {
    }

    ~Game()
    {
         if( tileset )
             delete tileset;
    }

    void LoadMap(const std::string& tileset)
    {
        if( !tileset )
            tileset = new sf::Image( "myImage.jpg" );
    }

private:
    sf::Image* tileset;
};


[Silvah]

If you cant initialise it immediately(see Laurent's example) I would use pointers(I would use them even with a default constructor, because it is very simple to check, weather a pointer is NULL or not and so you can prevent missuse of an uninitialised image).

You just introduced unnecessary dynamic memory allocation.

Honestly, I can't see how not having a default constructor could be better than having one which creates an empty image, and I can't see why a Clear() method would be so evil. Okay, it's possible to use an empty image, either by accident or intentionally. But think about it: in this regard, built-in types are much worse. You can use uninitialized int. You can use uninitialized pointer. The effect is quite bad: you end up with undefined behavior. On the other hand, at least some operations are well-formed for an empty image.

[Nexus]

Honestly, I can't see how not having a default constructor could be better than having one which creates an empty image

An empty image can be meaningful, this also has to do with Laurent's "policy" that image loading errors are no fatal errors (e.g. show a white sprite instead). But generally, it's certainly not bad to ensure valid instances when possible. This saves the programmer from logic errors and case differentiations.

and I can't see why a Clear() method would be so evil.

I think the question is rather if a Clear() method is really useful. There are hundreds of features which are not evil, yet which don't deserve an introduction into SFML because of their irrelevance. And I haven't ever felt the need for a sf::Image::Clear() function, to be honest.

But think about it: in this regard, built-in types are much worse.

True, but that's not really an argument. There is always something worse. Apart from that, sf::Image has much higher level semantics than built-in types, therefore it should provide more restrictions about value semantics and object validity.

[Silvah]

I think the question is rather if a Clear() method is really useful.

I was responding to Fred_FS and since he seems to claim that it'd be evil, based on the fact it's possible to misuse it (heck, it's C++, it's possible to misuse pretty much everything!), I wonder if it really is.
And if someone needs it, for a reason I can't think of, it exists, it's only spelled differently

[Fred_FS]

I am sorry. I did not know that SFML doesn't crash, if you try to use an "empty" image(I never tried it ). Hence it wouldn't be that bad to use a cleared image.
And of course the user is able to misuse almost everything in C++, but that is not a reason to offer functions that are unnecessary and risky.

My point is that if a user want to use such a function his design is probably imperfect. Just deleting some image information, but keeping an empty variable, is in my eyes bad design. So I just want to warn not to change a system that it meets the needs, but to consider weather the own needs are sensible.
And for this reason I would not provide a function to clear an image, because in many cases it would be misused(in that way that it is used in a bad design).

[JayArby]

Sounds like a lot of you are coming from the Java viewpoint, especially with those static factory methods.

In Java, all objects are pointers and null pointers are basically empty objects. In C++, it is completely normal to have stack allocated objects that are invalid until initialized. It's no different from Java in substance, because rather than testing for a null pointer, you convert the object to bool or call some method to test it, like file.isOpen();

Again, I'm fine with the Java way or the C++ way, but mixing them doesn't seem like a good idea.

It makes sense.
However I'm not sure it is really necessary; maybe you should give us an example of a situation where unloading an image while keeping the instance alive is really useful.

Well, here is my personal example: a static Image object is used to display a thumbnail view of the currently selected image in a list of image files. When no image is selected, there is no reason to keep the image in memory. So I want to unload the image so that the thumbnail display will automatically know, on checking the image, that nothing needs to be displayed. So I had to use pointers. No big deal, but it just doesn't seem like the Image class was really meant to be used like that. It would have been more elegant to use a stack allocated object.

[Nexus]

In C++, it is completely normal to have stack allocated objects that are invalid until initialized.

Certainly not, invalid objects are almost always bad programming style. In constrast to Java, C++ is able to enforce the initialization of member variables, which is a good thing. As mentioned, an empty sf::Image is not strictly invalid in this sense.

I know that some people use Init() methods instead of constructors and have status flags and case differentiations, but that's neither normal nor the good way to go.

So I had to use pointers.

You could use the assignment of an empty image.

[Fred_FS]

Sounds like a lot of you are coming from the Java viewpoint, especially with those static factory methods.

Are you talking with me? I am certainly not coming from Java. But I also like the way to use constructors to initialise my objects .

Again, I'm fine with the Java way or the C++ way, but mixing them doesn't seem like a good idea.

C++ offers pointer. So you aren't mixing anything. Avoiding dynamic memory allocation doesn't mean not to use it at all.

Well, here is my personal example: a static Image object is used to display a thumbnail view of the currently selected image in a list of image files. When no image is selected, there is no reason to keep the image in memory. So I want to unload the image so that the thumbnail display will automatically know, on checking the image, that nothing needs to be displayed. So I had to use pointers. No big deal, but it just doesn't seem like the Image class was really meant to be used like that. It would have been more elegant to use a stack allocated object.

I would prefer to keep the image in memory because unloading and loading images is probably not the best for your performance.
another way is to use an image loader. This image loader has a list, with all images, and if you need an image it checks if it is necessary to load the image or if the image is already in the list. If you don't need an image anymore, you can simply remove it from this list and you don't have any empty images.
but as I have already mentioned: I don't think that it is the fastest way, to unload and reload the images.

[JayArby]

Maybe "uninitialized" is the wrong word. "Empty" is a better word. It doesn't imply that the data wasn't initialized.

Example:


fstream myfile; // <- empty object

...

myfile.close(); // <- empty object

[JayArby]

so...I presume the answer to my feature request is 'no'?

[Laurent]

Absolutely