Exceptions in SFML 3 ?

[binary1248]

How would SFML benefit from a filesystem library? Only by making error messages a bit nicer?

I believe the other cases that are typical for games, such as archives, file dialogs, ... are too high-level for SFML anyway. There exist already portable and well-tested libraries such as Boost.Filesystem (and C++14 in the future), I don't think it makes sense to reinvent the wheel once more.

I was actually surprised to find out that C++ indeed could only open/create files, it couldn't delete files, or manipulate directories in any way. I don't think the question is whether we should implement something that is to come in other libraries anyway. Before SFML came around, there were other libraries that were already portable and well-tested, yet Laurent went ahead and started with SFML because he thought he could do better, and he was right. It is kind of hard to "do something better" than a standard library but that doesn't mean it can't be considered. It's about the seamless integration into what already exists in this case that makes something worth considering.

About Boost.Filesystem, maybe you might not have heard, but even though I consider boost a wonderful project and am glad many of it's features made it into standard libraries, I find it hard to use selected subfeatures of it isolated from others. This is why although boost would be the perfect solution to many problems I have, I normally resolve to more "lightweight" libraries that do exactly what I want and nothing more, and that is the direction SFML could take if this were to be considered.

There are so many fundamental things that C++ programmers don't know aren't possible with the standard library. The best example would be directory creation. To create a directory, either you resolve to writing platform specific code for all platforms, or you get some other heavyweight library like boost. SFML should be a toolbox of independent modules (much like it already is) that helps you to get your day-to-day tasks done without much effort or overhead, and filesystem support is one of those things if you ask me.

[Ixrec]

Admittedly, considering how many super simple filesystem things I've done in my own program with a single #ifdef, someone could probably list a bunch of them in a single header file and put it on the wiki.  Should we start a separate thread to try and compile a good list of useful ones?

As for actual SFML changes, the only one I really want is changing loadFromFile so it doesn't demand a std::string for the filename.  As it stands I have to use standard library filestreams and loadFromMemory to load images with non-ASCII filenames on Windows, which is a bit annoying.  And yes Windows is inferior to Linux for not using UTF-8 and all that, but still, that's a detail I'd like to not have to think about.

[Tank]

In fact, it removes some flexibility in the user code style.

Can you name the removed flexibility?

Should SFML force the user to use factories? I don't think so. There are too many design out there to force the user to use X (where X is not that much linked to C++ style like exceptions are).

The question is: How much different -- in terms of usage -- is it to either do image.loadFromFile() or Adapter::loadFromFile()? image.loadFromFile() is not a factory method, as it does not create things, but that's probably the only line of code that's really different. In contrast that means you get separation from data and logic (image data/image loading) without being really different in how things work.

I really agree to MorleyDev when he says: »[...]I find it conceptually simpler that an Image is simply an Image, all it does is be an Image and all it knows is how to be an Image. Not create one, not decode one from a PNG or JPEG file, just be an Image.«

It's what I personally find to be easiest to use. Being biased by test-driven development, I'm also thinking about how tests would look like for classes like sf::Image. For me it'd be weird to write image loading code for sf::Image, I would not give sf::Image any other task than holding an image and allowing modification of it.

It's not that I dislike the current design. It does what it does, and it's simple enough to be understood even by beginners. And it's of course still clean, no doubts. MorleyDev had to make this clear, so do I: I'm only elaborating ideas, especially those I'd consider myself. Also I did not want to »attack« SFML with my GDD article. The idea for writing it came into my mind when eXpl0it3r showed up that one assert() in the SFML book can't be avoided (there's a thread somewhere in this forum). Then I remembered that I had a hard time thinking of a consistent error reporting strategy for myself. Of course the article relates to SFML, because I'm all »go away error codes«, but it was not written especially for it.

But what I really don't get is the »exceptions AND error codes« discussion. Why do we need error codes if we have exceptions? Can someone sum this up again for me in short points? If I don't care of errors, I do not try/catch -- and my program will crash for free if an error happens anyway. If I care, I simply use try/catch instead of if-then-else (for error codes).

If I browse over code and see a try/catch somewhere I already know, "yeah something really bad and unexpected is going to happen somewhere around here" and that is not a feeling I'd like to live with. It's like walking through a city where everyone is wearing a gas mask as opposed to one that has a government that prevents dangerous chemicals from being recklessly disposed of (bad example I know but hopefully you get the point ).

Speaking of myself, I wouldn't like the feeling of walking around in a city where nobody is wearing a gas mask when the possibility of a gas attack is known and given. If there are known issues that can't be avoided, then I'm really happy to see that someone's able to handle them properly instead of letting the boot sink.

[wintertime]

As for actual SFML changes, the only one I really want is changing loadFromFile so it doesn't demand a std::string for the filename.  As it stands I have to use standard library filestreams and loadFromMemory to load images with non-ASCII filenames on Windows, which is a bit annoying.  And yes Windows is inferior to Linux for not using UTF-8 and all that, but still, that's a detail I'd like to not have to think about.

Thats actually more like a deficiency of the C library on Windows. The problem is the filenames get treated as ASCII and not as UTF8, but cause of some lucky circumstances UTF8 uses the same encoding for characters up to 127.
To fix this shortcoming (maybe it could also be called a mostly silent bug) SFML would have to treat the std::string as being UTF8, then convert it into widestring and use some function that accepts these for handling files. Or maybe it is also possible to change the codepage from C to UTF8, but there I'm not so sure its supported from Windows. http://utf8everywhere.org/
Now to repair this there is the possibility many resource classes would have to change their load methods, if there was only a single outside function there would be only a single point of change.

[Ixrec]

I definitely get that the problem is Windows' fault and not SFML's (hence the "inferior" comment) but it's still something SFML could probably hide from the user.  Since I managed to work around it just by using the standard library fstreams and loadFromMemory, I'm sure it can be done somehow.

In fact, I'm adding that to my mental list of stuff I want to contribute to SFML when I'm finally in a position to do serious coding again.

[Hiura]

In fact, it removes some flexibility in the user code style.

Can you name the removed flexibility?

Should SFML force the user to use factories? I don't think so. There are too many design out there to force the user to use X (where X is not that much linked to C++ style like exceptions are).

The question is: How much different -- in terms of usage -- is it to either do image.loadFromFile() or Adapter::loadFromFile()? image.loadFromFile() is not a factory method, as it does not create things, but that's probably the only line of code that's really different. In contrast that means you get separation from data and logic (image data/image loading) without being really different in how things work.

I guess you're quoting me and not Nexus so I'll answer. 

Indeed, Image::loadFromFile() is not a factory method. That's part of my point. With SFML 2, you can do this:

sf::Image img;
img.loadFromFile(...);

or you can write your own factory method, with his prefered design and all, that internally use loadFromFile():

sf::Image img = Factory::image(...);

You can do both and the user is not forced to use one or the other. The key point is: SFML gives the tools to the user so he can do whatever he wants.

If we split the loading from the image class itself, we remove the possibility of using a method and force the user to use a factory. And since some people don't like factories, some flexibility for them is removed. One could create a wrapper to add (intrusively) a loadFromFile method but this would be completely madness. On the other hand, one can create (non-intrusively) factories.

Also, there are more than one pattern for factories. Which one is the best? Well, it's a purely subjective opinion. Hence we will fail to create the factory for everyone.

Beside flexibility, there I also have another (minor) concern: consistency. Many classes have many parameters and the current design propose simply a very basic constructor that initialize only the major attributes of the class (*Shape, Sprite, Text, Transformable, ...). Then the user call, e.g. setOutlineColor(), setFillColor(), ... Basically, the whole Graphics module works like that. It feels right to use loadFromX. I agree it's not the argument against splitting stuff into different components but still it add some weights on the balance.

But what I really don't get is the »exceptions AND error codes« discussion. Why do we need error codes if we have exceptions? Can someone sum this up again for me in short points? If I don't care of errors, I do not try/catch -- and my program will crash for free if an error happens anyway. If I care, I simply use try/catch instead of if-then-else (for error codes).

It's not necessarily about caring or not about the error itself but about coding style – do you want to use try-catch of if-else when you're looking for errors?

From my point of view, exceptions are enough for a C++ app. We argued above that's «the C++ way».

But some (deservedly) pointed out some issue with bindings. Since bindings will continue to exists, it make sense to provide in SFML itself a way of using error instead of exception and thus provide a nice bridge for C-based bindings.

[Ixrec]

---On splitting classes like sf::Texture:

On top of my personal opinion that SFML is far more [insert a few dozen vague positive coding adjectives] with a single sf::Image class than it would be with a separate loader class, there's a more pragmatic issue no one's discussed yet.

If we really took all the image manipulation out of sf::Image and into some other class, wouldn't sf::Image just be a glorified wrapper for a Uint8*?  And then the "some other class" would just be sf::Image under a different, less intuitive name, wouldn't it?  So regardless of any theoretical cleanliness the whole idea seems like it wouldn't amount to anything.

I think one of you will have to write down the exact "split API" you have in mind in order for this discussion to go anywhere new.

---On exceptions AND error codes:

@Tank: The rationale there has been explained before but this thread has gone on long enough I guess it got lost.  Basically, loadFromFile is one of the few functions where we can assert two sets of expectations, such that each one will apply in some but not all use cases, namely:
- The desired file will load unless something truly exceptional happens.
- The desired file may or may not load, and we have a fallback plan in case it doesn't, because that's not exceptional at all.
The idea behind providing both exceptions and error codes is that we want to support both use cases.  If we allowed only exceptions, we would be forcing all users to code as if file loading errors are truly exceptional problems, even if that's not the case for them.

I should emphasize that this seems to be the only function for which this argument has real weight, so the proposal is that just this one function (well, all the different versions of it) would have the error codes option, and that would have to be explicitly enabled or chosen somehow.

[FRex]

I was actually surprised to find out that C++ indeed could only open/create files, it couldn't delete files, or manipulate directories in any way.

C++ includes C, does std::remove from cstdio not work for removal?
http://www.cplusplus.com/reference/cstdio/remove/

[binary1248]

C++ includes C, does std::remove from cstdio not work for removal?
http://www.cplusplus.com/reference/cstdio/remove/

I can't remember if it worked for me, but when I needed this functionality it was a while back and I couldn't find a portable solution. Maybe it might work for me now...

[Tank]

I guess you're quoting me and not Nexus so I'll answer.

Oops, indeed. I guess I mixed up the avatars. AFAIR Nexus used it before as well.

You can do both and the user is not forced to use one or the other. The key point is: SFML gives the tools to the user so he can do whatever he wants.

I don't agree on the attitude. In my opinion a library should choose one way of doing things and go for it. If you're arguing that making all the users happy because they can use the library how they would like it to be used, then I'm against that. My opinion in general is »You have to learn how something works to be able to use it« instead of »What you're going to use has to work like you want it to be«.

Example: Vim, Blender, text shells. A lot of people find all these things hard to use and not intuitive. However if you learn their way of doing things, you will be very effective (that does not make them easy to use or intuitive, but still effective).

If we split the loading from the image class itself, we remove the possibility of using a method and force the user to use a factory.

If there are reasons to do so (which there are from my point of view), I'm fine with forcing people to do it right instead of providing them with options just to satisfy habits (if that really is the case; this needs research and discussions, just like this one).

One could create a wrapper to add (intrusively) a loadFromFile method but this would be completely madness. On the other hand, one can create (non-intrusively) factories.

That's true, and it seems to be a clean way at first. Otherwise, you have no option to get loadFromFile() out of sf::Image in any way, it's hardcoded into the class (which also forces me to something). In my eyes it's problematic because the loading routine is not an extension, it's core functionality. I'm quite sure that we won't be having problems with sf::Image::loadFromFile() at anytime in the future, but if it's going to make trouble, then sf::Image might be an issue in general.

Besides of that, I don't know what the loadFrom* functions do to the object in detail, because they have full access. Again, this might not be an issue with sf::Image specifically, but if we can even avoid the questioning, then it's a benefit I think.

Then the user call, e.g. setOutlineColor(), setFillColor(), ... Basically, the whole Graphics module works like that. It feels right to use loadFromX.

But that's exactly what I'm pointing out.. The setters and getters modify/give access to the data itself, which makes perfect sense. For example, calling setPixel() or getPixel() on sf::Image is correct. load*() is completely different, it's functionality that has nothing to do with the data itself. It's an operation that does some random stuff to create/initialize an image. The image itself however does not care about that at all, it's unrelated.

do you want to use try-catch of if-else when you're looking for errors? ... But some (deservedly) pointed out some issue with bindings. Since bindings will continue to exists, it make sense to provide in SFML itself a way of using error instead of exception and thus provide a nice bridge for C-based bindings.

It's not important what one wants to use, it's important what makes more sense. Regarding bindings: I stay at my opinion that a C++ library should be written in C++. If SFML would be a C library, for example, and I was going to add a C++ layer to it, then I'd add exceptions on top. Just like CSFML can transform them into whatever makes sense in C.

The idea behind providing both exceptions and error codes is that we want to support both use cases. If we allowed only exceptions, we would be forcing all users to code as if file loading errors are truly exceptional problems, even if that's not the case for them.

In my opinion not users define what errors are exceptions, the library does. If loading a file is exceptional to a loadFromFile() function, which makes sense, because it's an operation that can't be checked for validity before, and by failing to load a file, the function fails to fulfill its use (loadFromFile, without loading from file, is a function that's..well..useless ).

[Nexus]

Before SFML came around, there were other libraries that were already portable and well-tested, yet Laurent went ahead and started with SFML because he thought he could do better, and he was right. It is kind of hard to "do something better" than a standard library but that doesn't mean it can't be considered. It's about the seamless integration into what already exists in this case that makes something worth considering.

I just think we could really invest our time into something more meaningful. A filesystem risks to suffer the same fate as sf::Thread: It won't be powerful enough to cover all needs, but at the same time a standard solution can easily replace it.

If we plan to integrate a filesystem, we should definitely not ignore the standard library and invent our own wheel. Personally, I always hate it when a library comes with its own STL, which I'm forced to use although it's worse. Instead, we can consider how to integrate the standard filesystem as smoothly as possible.

Otherwise, you have no option to get loadFromFile() out of sf::Image in any way, it's hardcoded into the class (which also forces me to something). In my eyes it's problematic because the loading routine is not an extension, it's core functionality.

But given its core functionality, it would rather be strange to extract it from the core (sf::Image) to a separate class, wouldn't it? Or which core are you referring to? The loading functions are clearly specific to the respective resource classes, you can't have a generic loader in SFML.

Besides of that, I don't know what the loadFrom* functions do to the object in detail, because they have full access. Again, this might not be an issue with sf::Image specifically, but if we can even avoid the questioning, then it's a benefit I think.

As mentioned previously, a separate class would also require full access through friendship, unless you add a constructor/method to initialize an image -- but then the API is again inconsistent, because some initializations require a factory and some don't. And again, this is only a superficial design issue, not a real problem.

Maybe you overlooked my post before, but I'd really be interested in concrete advantages of a separate ImageAdapter class that would justify the drawbacks (more complicated API and implementation, more boilerplate code to write, maintain and debug). What does the user effectively gain from such a modification?

I guess I mixed up the avatars. AFAIR Nexus used it before as well.

No, I only use my own creations (previously the Thor logo)

[wintertime]

Otherwise, you have no option to get loadFromFile() out of sf::Image in any way, it's hardcoded into the class (which also forces me to something). In my eyes it's problematic because the loading routine is not an extension, it's core functionality.

But given its core functionality, it would rather be strange to extract it from the core (sf::Image) to a separate class, wouldn't it? Or which core are you referring to? The loading functions are clearly specific to the respective resource classes, you can't have a generic loader in SFML.

This made me curious now and I directly looked at the SFML source. Guess what I found?
The load methods are in no way integral to sf::Image, they just forward the call to the sf::ImageLoader class already with 2 references to store the decoded data and x,y. Btw. ImageLoader is a singleton without having a need to be one, it could just as easily be a class with only static methods or a namespace as it got no member variables.
There is also a SoundFile class already.
And the Shader class got some external helper functions getFileContents and getStreamContents in a private namespace that seemingly could be moved somewhere to be reused.

There is some code duplication in the underlying helper functions that could be avoided, for example loading from file does not just load a file and reuse load from memory but got this intermingled and replicated. If loading without decoding was done by a single function for all of SFML and then the different resource classes would just have functionality for decoding from memory there could be a reduction in internal code.

[Ixrec]

Something tells me at least some of those oddities have a long justifiable story behind them.  Would be cool to hear what Laurent has to say about them (if anything).

I notice we still have 0 responses that argue for any practical benefit of splitting up the classes.

[Laurent]

Something tells me at least some of those oddities have a long justifiable story behind them.  Would be cool to hear what Laurent has to say about them (if anything).

You're right that these classes have an history. But they also have good reasons to remain as they are.

ImageLoader exists so that the sf::Image implementation is cleaner and not bloated with external libraries loading routines -- ImageLoader.cpp is already bigger than Image.cpp. So yes, the separation makes things clearer and more maintainable, but this doesn't mean that I should make all this stuff public. And it's a singleton because in the past there were some external library init/cleanup functions to call, and that may be the case again in the future so I leave this class as a useless singleton

SoundFile exists for the same reason. It might be public one day, but for a totally different reason: it provides a feature that is not available in the public interface (accessing a sound file's samples without decoding and storing them all in a sound buffer).

Utilities to read a stream or a file in memory are just implementation details that were refactored because they are used at several places in the class, there's no reason to put such tiny straight-forward generic functions somewhere in the public API.

There's something else I'd like to say: loading from a file or from a stream is not just loading the file or stream in memory and then loading from memory. Each resource (through the corresponding 3rd-party library) has its own optimizations for each kind of loading. Most resources can be read and decoded at the same time, thus removing the need for an intermediate dynamically allocated buffer, and possibly making the whole process faster -- which matters, since resources can be big and slow to load.

And don't forget resources that are streamed: sf::Music, that everyone already noticed, but also sf::Font. So a decomposed way of doing things would definitely not work globally, thus creating inconsistencies in the API.

[binary1248]

...

This is why SFML has become so successful, because Laurent focuses on pragmatic issues. Form follows function, and if experience has shown that some theoretical idea might not work out in real life then there is no point trying to force people to reduce their productivity just so that something is "theoretically" more clean. A lot of theory has gone down the drain because they couldn't prove themselves (look at the OOP hype, was much worse when it came out), and maybe in 20 years from now we will look back at these ideas and wonder how we could be so crazy .