-
This topic was originally following this discussion (http://en.sfml-dev.org/forums/index.php?topic=9711.msg92672#msg92672).
It's not bool or err(), it's usually both ;)
And yes, SFML 3 will most likely use exceptions everywhere.
-
Ah, sorry. I only started redirecting sf::err() recently because I wanted to see shader compiler errors, so I hadn't noticed loadFromFile uses it too.
-
And yes, SFML 3 will most likely use exceptions everywhere.
Conditionally plz. :-X Even boost filesystem has support for both codes and throwing. Exceptions are too oppressive to be used everywhere. :(
-
What are your arguments against exceptions? Why are they oppressive?
-
1. Screw with C code callbacks, ie. when getting thrown in Lua wrapper when Lua wasn't compiled as c++(not the brightest idea, but doable).
2. Screw with literally every language, so now C wrapper for porting has to catch exceptions and return error code and then let wrapper language do something sane(throw? error code? `rm -rf /` ?) with that code.
3.The fuss of converting error code to exception is way way lesser than fuss of converting exception into error code:
Code to throw:
if (tex.loadFromFile("pic.jpg")) throw loadFromFileFailed();
One liner, literally.
Throw to code:
try
{
tex.loadFromFile("pic.jpg");
}
catch(loadFromFileFailed& e)
{//by this point I am already kind of discouraged and want to go play with some other library that doesn't force exceptions
return errcode::loadFromFileFailed;
}
No comment, looks stupid, definitely can't be faster, might be slower, pointless as hell.
4. Consider use case of an imaginary unix-y style tool for checking your ready-to-deploy zip/pack/bundle/game.laurent file/folder to ensure all textures load ok in SFML, takes a list of texture paths on stdin and outputs them with info about SFML compatibility on stdout:
int main()
{
std::string name;
sf::Texture tex;
std::cout<<std::boolalpha;//pretty bool printing
while(std::getline(std::cin,name))
{
std::cout<<name<<"loadable via SFML:"<<tex.loadFromFile(name)<<std::endl;
}
}
With everything-is-throwing-cause-modern-cpp-is-one-size-fits-all:
int main()
{
std::string name;
sf::Texture tex;
std::cout<<std::boolalpha;//pretty bool printing
while(std::getline(std::cin,name))
{
try
{
std::cout<<name<<"loadable via SFML:";
tex.loadFromFile(name);//returns void but can throw loadFromFileFailed
std::cout<<true<<std::endl;
}
catch(loadFromFileFailed& e)
{
std::cout<<false<<std::endl;
}
}
}
Consider based on 3. and 4. how much harder it can be to come up with use case where error code you'd prefer to be exception is more cumbersome than exception that you don't want(stupid programmer who ignores returned codes and then has issues about his textures being white and sounds missing because loads actually failed and returned false he didn't check for doesn't count, he could as well ignore result of malloc and have a memory leak, functions return for a reason, he will pray exceptions don't happen or make giants try{}catch(...){} blocks and then have the same problem).
5. Screw with C resources, almost every C library is a c++ one:
some_c_resource * resource=open_something_c();
sf::Texture tex;
//lets say we use that resource to look up name of texture we want
tex.loadFromFile(look_up_value_of_attr_as_str(resource,"texname"));//BTW all your C resources are belong to Laurent if this throws and you don't catch it here, close them and then rethrow, hehe, happy boilerplate writing <3
5.1. Yes, you can write wrappers or use template parameter deleter in smart ptr to close C resources OR write non throwing api that returns codes around this one. You probably should wrap your C stuff when you're forced by real user case of your code or want to make your work easier, not by arbitrary design choice of a library made to be more 'modern'. You're fighting the library at this point, go pick up SDL or GLFW or something else.
6. They are explicitly designed to take control away from my code and so they introduce boilerplate and make me fight the library if I don't want something like a texture manager to throw, and even if I did, they'd be my well organised coherent exceptions, not thor and sfml going to town on my scopes with over generalized exceptions thrown from too low to have enough context, 'textureloadingfromfile' failed, yep, I'll totally catch that 10 places in call stack from now on, and totally distinguish that certain texture loading from another one in another module at another time... It'll end up being ignored or used one place up just like an error code would.
A texture manager should just get given a list of data handles that are from disc, ram, wherever via inputstream interface, load texture from each and report how many it loaded, it's not supposed to be verifying anything. If needed it's state(amount of textures actually loaded) can be checked by higher system and then it can throw if needed. Maybe this higher system that uses this class isn't so throw trigger happy, maybe it's that unixy tool that will just check if all textures load ok. Don't assume I want to bail scopes on all failures(because with everything-uses-exceptions you do assume just that, that or that I want to write additional empty catch in case I don't want to bail..).
I really dislike exceptions overuse and find them annoying unless they're done like Lua, Boost and/or used sparsely when it makes sense.
-
Wow...should we make a separate thread for debating error handling best practices?
Personally what FRex says feels right specifically for loadFromFile, but when I actually think about it there really aren't any typical use cases where a failure to load a file has an obvious and simple alternative within the current function that preserves normal execution flow.
I can think of exceptional use cases, and cases where alternatives would exist higher up the call stack, but of course those are precisely the cases where exceptions are clearly better than return codes.
P.S. I don't understand bindings to other languages enough to have an opinion on that part of the issue.
-
P.S. I don't understand bindings to other languages enough to have an opinion on that part of the issue.
You can't propagate exception to any foreign language, some have very different exception strategies or not have them at all(C) so it's performance hit without any benefit + it affects a binding which is inherently at a disadvantage cus it's calling c++ through C wrapper = awesome. Sometimes even trying to pass it in C might make mini-hiroshima in your program, on MinGW DW2 is 32 bit only, bloats the executable, is much faster but breaks winapi callbacks, SJLJ is way slower(10%-30% hit in 'exception heavy code', but it's found on internet so take it with a grain of salt), more space efficient and doesn't break callbacks.
Personally what FRex says feels right specifically for loadFromFile, but when I actually think about it there really aren't any typical use cases where a failure to load a file has an obvious and simple alternative within the current function that preserves normal execution flow.
Then throw. You can have map of strings to shared pointers, and if loading fails you can assign some very eye sore texture to catch erorr in game(I actually did that irl at one point and plan to do it again after I stop using debug draw from Box2D as my ad-hoc graphics), or throw up and crash entire game. In case you want to throw it's one line for you, in case I want to not throw it's 4+ lines + small performance hit for me.
Also the unix-y program example above wouldn't look good with throws, as shown.
Just keep loading, have a variable to count, do m_successfulloads+=m_map[name].loadFromFile(path); and then check if amount of passes paths is == to m_successfulloads somewhere else or even throw there if any load fails, include name or part of name or index of missing path or something in your exception(but don't allow it's ctor to throw too). 'FailedToLoadFromFile' exception is nearly useless on it's own, which texture was it? What path did it have, what module was it being loaded etc. it's thrown too low to be useful, you can't catch it very high or you wont know which module threw it or have to poll them all and ask if any of them recently had a texture load cycle that got interrupted -> awesome...
If it was thrown even one level higher in method that loads all paths, it could include part of the path and then no matter where you caught it, even in main, you would know that red_splash.png must have been thrown from particle module, hero.png is from sprites module and adorable_katzilla.png is from PrettyKitty module, or you might throw different exceptions with different what() altogether for different modules. Non optional exceptions are annoying, require boilerplate to work around and might become very slight(but still existent..) performance hit.
-
To use C# as an example, Parse vs TryParse. Parse throws on failure to Parse, TryParse returns true/false. Is this problem not solved by creating tryX functions to create functions for whom failure is not exceptional.
For loadFile vs tryLoadFile, that actually feels significantly better to me. loadFile says "I want to load this file" and failure is an exceptional scenario, and with tryLoadFile it's saying "I want to load this file but not guarantee it will work".
Exceptions are for exceptional situations that deviate from the expected control flow, tryLoadFile is explicitly including both success and failure in it's expected control flow, whilst loadFile failing to load the file is an exceptional situation.
-
To use C# as an example, Parse vs TryParse. Parse throws on failure to Parse, TryParse returns true/false. Is this problem not solved by creating tryX functions to create functions for whom failure is not exceptional.
For loadFile vs tryLoadFile, that actually feels significantly better to me. loadFile says "I want to load this file" and failure is an exceptional scenario, and with tryLoadFile it's saying "I want to load this file but not guarantee it will work".
Exceptions are for exceptional situations that deviate from the expected control flow, tryLoadFile is explicitly including both success and failure in it's expected control flow, whilst loadFile failing to load the file is an exceptional situation.
Describing the different interfaces through their function names is a relic from C times. In C++ (and C#) overloading was introduced to solve that problem. Having TryX functions instead of just providing a transparent overload does not conform with modern C++ style. Some people argue that it might not be clear what to pass to the function without reading the documentation. In that case, the interface is bad anyway and should be changed to something self-explanatory.
There are 2 scenarios:
#1: The user expects success and would exit with a fatal error anyway if the current interface returned something that denotes failure. For this, exceptions are just right and even easier to use than the current interface:
// Old way:
if( !loadFile( name ) ) {
std::cerr << "Fatal error: Couldn't load file " << name;
return 1; // Exit the application
}
// New way:
loadFile( name );
// If this fails, and the user doesn't catch the exception, the application will close with a hopefully descriptive error message.
// terminate called after throwing an instance of 'sf::file_load_failed'
// If you wanted, you could catch the exception at the end of your main() and make it print more useful information via .what() and rethrow it.
#2: The user cares about whether the file was loaded or not and execution of the application can continue albeit in a slightly different manner. This would be the case if the if-block was triggered with the current interface:
// Old way:
if( !loadFile( name ) ) {
std::cerr << "Couldn't load file " << name << ", oh well... now you won't be able to view its contents.";
}
// Continue execution...
// New way:
sf::Error error;
loadFile( name, error ); // optionally declared noexcept, error is passed by reference
// The user can check for the error because they might do something differently based on it.
if( error == sf::Error::FileLoadFailed ) {
std::cerr << "Couldn't load file " << name << ", oh well... now you won't be able to view its contents.";
}
// Continue execution...
This is already done in many C++ libraries. I personally use a mix of both. Sometimes you need scenario 1 functionality and sometimes you need scenario 2 functionality, but at least there is a way to specify which you want. This also frees up the return value for more "useful purposes".
-
Except here my argument is these are not the same function, that they have different responsibilities and as such deserve different names. Overloads exist to present a common responsibility with multiple interfaces, not to present multiple responsibilities with a common interface.
tryLoad says failure can be expected so failure is not exceptional, which is discrete from load which says success should be guaranteed and thus failure is exceptional.
I will grant the error pattern you present may be established in C++ for handling error conditions in situations where errors are exceptional but users do not want to use exceptions for various reasons. But here I present an alternative which is to create a tryLoad that acknowledges failure as not exceptional, and put forth that as a cleaner solution that more closely matches the needs of the user.
That is my argument, although I'll grant it hinges on whether or not you think there should be a version of loading that doesn't regard failure as exceptional.
As an aside, can I say I do love that these intellectual design discussions crop up in the SFML community with some regularity. It's nice when sometimes it can seem there are so few programmers out there who value thinking about this kind of thing. The debate is always fascinating.
And to be honest I have other issues with the current Texture/Image/Shader loading, namely I disagree that any of those should know how to load themselves from the file system, that it should not be their responsibility.
-
Well, if we're gonna keep having this debate in this thread...
I definitely disagree with the idea that classes shouldn't know how to load themselves. That kind of simplicity is what makes SFML so incredibly useful. I think you could make a case for a non-member function to do all of that like sf::loadFromFile(sf::Texture& tex, sf::String filename). On the other hand, where would you put the overloads of this function?
It seems like the main point we all agree on is that loadFromFile at the very least should have an option to return an error instead of throw an exception. Can anyone think of any other functions where we can make a strong case for two sets of expectations?
Regarding how the option would be implemented, I agree with binary that it should be a transparent overload, and that the exception-on-failure option should be the default single-parameter overload. I'm not sure what I'd want the second parameter to be though, and I think that would partly depend on whether there are enough other functions with this option to warrant designing an sf::Error struct (union?) to act as the extra parameter in these cases.
-
Well in code they don't actually know it, but they appear to do so from the API point of view. That, to me, is a big hint that the responsibility is in the wrong place: SFML Image has feature envy of the ImageLoader. I'd rather an ImageLoader be an explicit class I have to initialise and use. To me, that is simpler. I definitely think it's easier to use and test code that separates use from construction like that.
I can't think of anything else in SFML that has two different expectations like loading does. Creating a texture, maybe? It's the things that do something and return true or false as a way of reporting success or failure, which are rare in SFML anyway. With an exception of pollEvent., of course.
-
I'd rather an ImageLoader be an explicit class I have to initialise and use. To me, that is simpler. I definitely think it's easier to use and test code that separates use from construction like that.
I disagree, this adds user-unfriendly complexity without real advantages, apart from theoretically "nicer" OOD.
Even this one is questionable: The SFML resource classes already have the knowledge about low-level resource handling and access to other libraries, be it for loading, preparing, playing or unloading. It doesn't make sense to split half of this functionality into a separate class.
I can't think of anything else in SFML that has two different expectations like loading does.
Maybe connecting sockets and sending/receiving packets? This is however more complex because of two operation modes (blocking and non-blocking).
By the way, it would be nice if Laurent could split the discussion :)
-
I do not at all see why that would be any simpler, and here's the fake code I'm imagining to prove it:
Current:
sf::Texture tex;
if(!tex.loadFromFile("Mario.png"))
throw my_exception("Couldn't find Mario.png");
Your proposal (?):
sf::Texture tex;
sf::TextureLoader texl;
if(!texl.loadFromFile("Mario.png"))
throw my_exception("Couldn't find Mario.png");
tex = texl.extractTexture();
Off the top of my head I have no idea what other features an sf::TextureLoader might have that aren't already in sf::Texture.
To put this in super-generalized theoretical terms (or at least the dialect of them that I'm using in my head right now): image loading is a task. It's not an object to be acted upon, but a task to be performed. One task means one function. The object that image loading acts upon is the object the loaded image is used in, be it an sf::Image or an sf::Texture. Thus, image loading should be done by a function that acts on sf::Image or sf::Texture. At that point it's just a question of member or non-member function. For sf::TextureLoader to ever possibly be a Good Thing(TM), we need to come up with some functionality it can have other than a slightly more complicated interface to sf::Texture.loadFromFile with the exact same behavior.
Now present the fake code you're imagining so we can dig into the details. =)
By the way, it would be nice if Laurent could split the discussion :)
Yeah, at first I was trying not to post in this too many times for fear of annoying someone, but since other people kept posting I figured he thought it was okay.
-
I'm cool with this going into a new thread if people want. I mostly just said the whole "image loading" thing as a way of highlighting my attitude towards design and programming, namely I prefer very discrete responsibilities in code. I'm also big on testability, for me simpler code is code that's easier write unit tests with and for. Code, frankly, cannot be simple without being testable for me.
Essentially, it appears things like create and loadFromFile were made because, due to the fact they could fail, they're reasonably complex and SFML atm doesn't use exceptions, people didn't want them in the constructors. But that, to me, doesn't mean move them to a member function. It means move them out the class.
If you look at the implementation of Texture.loadFromFile, it just calls through to sf::Image. At present every usage of it could be inlined and since it's pretty much just a helper done to hide the need of sf::Image it literally would inline to:
sf::Image image;
auto success = image.loadFromFile("filename") && texture.loadFromImage(image);
It's a helper function, nothing more. My preferred usage would be more like:
sf::ImageLoader imageLoader;
sf::Image image = imageLoader.loadFile("mario.png");
sf::Texture texture(image);
And loadFile throws on failure. Or does whatever the above debate will yield, Exceptions with an overload that takes sf::Error seems to be the preference. And I'll admit I do not mind that approach.
Really this is more about the whole self-mutating nature of image/texture, I find it preferable for images be created in their desired state and changing the image requires reconstructing the object.
But I always just create my own factories that look a lot like the above for me anyway, so it's a minor design nibble in the end that doesn't get in the way and I didn't intend for my aside comment to distract from the main topic. For that I apologise.
-
Throw to code:
try
{
tex.loadFromFile("pic.jpg");
}
catch(loadFromFileFailed& e)
{//by this point I am already kind of discouraged and want to go play with some other library that doesn't force exceptions
return errcode::loadFromFileFailed;
}
definitely can't be faster
Of course it can be.
If the exception handling method is a so-called "free" exception handling, like DWARF2 or 64-bit SEH, then in the usual case, i.e. no exception is thrown, the cost of setting up a try/catch is zero. As in, no code has to be run, for all the stuff required is in the static data already.
On the other hand, the reverse conversion, if(result == ERROR) { throw something; }, requires a conditional branch. Yes, it may will be well-predicted, making it cheap, but even if it is, it will pollute the branch predictor history, possibly evicting a branch that occurs in the hot path from there. And sure, that hot branch will eventually get there back again, but for the first few times, it's more likely to be mispredicted. The cost of mispredictions is typically measured in tens of cycles, which sure doesn't sound like a lot, except it's already way more than the cost incurred by try/catch in the aforementioned case.
There's also the question of the callee detecting the error. Perhaps surprisingly, it's the same in both cases: it's if(someLowerLevelStuffResult == ERROR) { propagate the error to the caller; }. Given the usual behavior is for no error to occur, it's a simply a not taken branch.
Obviously, if the expected behavior is for an error to actually happen, then the above considerations don't apply, but something is probably amiss anyway.
on MinGW DW2 […] breaks winapi callbacks, SJLJ […] doesn't break callbacks.
It's the other way around. No, really. It's the other way around.
If one throws across callbacks while using DWARF2 unwinding, the unwinder has no idea whatsoever what to do with that foreign stack frame for which there are no unwind data available. So it calls std::terminate.
If one throws across callbacks while using setjmp/longjmp unwinding, the unwinder has no idea whatsoever about anything, its only job is to longjmp to the nearest matching handler. If there is a matching handler, it blindly longjmps to it, completely ignoring any possible cleanup code that the foreign code would otherwise execute. If the foreign code acquired some resources, they're leaked. If it held a lock, a deadlock awaits.
To sum up: with DWARF2, you get a loud immediate crash. With setjmp/longjmp, you get a silent corruption. I'd take the former over the latter any day.
To be completely safe, don't ever throw across foreign code.
EDIT: fixed a typo.
-
Of course it can be.
Zero cost doesn't make it faster, just equal, I knew someone would come running with no cost argument or something so I wrote it "can't be faster" not "it's slower".
It's the other way around. No, really. It's the other way around.
No, it's not unless all these pages are horribly outdated, contain 100% false information and during last 2-4 years everything got put on it's head:
1. http://tdm-gcc.tdragon.net/quirks
2. http://www.mingw.org/wiki/GCCStatus
3. http://gcc.gnu.org/wiki/WindowsGCCImprovements
4. http://sourceforge.net/apps/trac/mingw-w64/wiki/Exception%20Handling
And here is what I wrote, in red you have link in which the same information is given:
DW2 is 32 bit only(1 and 4), bloats the executable(1 and 3), is much faster(1, 2 and 3) but breaks winapi callbacks(1, 3 and 4), SJLJ is way slower(1, 2 and 3), more space efficient(1 and 3) and doesn't break callbacks(1, 3 and 4).
If there is a matching handler, it blindly longjmps to it, completely ignoring any possible cleanup code that the foreign code would otherwise execute.
According to 3. it's `common` to throw in callback and catch in win event loop.
On the other hand, the reverse convertion, if(result == ERROR) { throw something; }, requires a conditional branch.
We are really getting into stupidly low level things now..
SFML would do such condition if it were to throw instead, stbi, ifstream, FILE * and GL all return error codes not throw. And SFML already has many branches, even in Texture.cpp itself, that are false for good data.
-
It's the other way around. No, really. It's the other way around.
No, it's not unless all these pages are horribly outdated, contain 100% false information and during last 2-4 years everything got put on it's head:
1. http://tdm-gcc.tdragon.net/quirks
2. http://www.mingw.org/wiki/GCCStatus
3. http://gcc.gnu.org/wiki/WindowsGCCImprovements
4. http://sourceforge.net/apps/trac/mingw-w64/wiki/Exception%20Handling
And here is what I wrote, in red you have link in which the same information is given:
DW2 is 32 bit only(1 and 4), bloats the executable(1 and 3), is much faster(1, 2 and 3) but breaks winapi callbacks(1, 3 and 4), SJLJ is way slower(1, 2 and 3), more space efficient(1 and 3) and doesn't break callbacks(1, 3 and 4).
I only refuted the part about "breaking in presence of foreign frames", the rest may or may not be true, my argument is not concerned about that.
If there is a matching handler, it blindly longjmps to it, completely ignoring any possible cleanup code that the foreign code would otherwise execute.
According to 3. it's `common` to throw in callback and catch in win event loop.
This may or may not be true, but honestly, I don't understand how this relates to what I said.
Of course it can be.
Zero cost doesn't make it faster, just equal, I knew someone would come running with no cost argument or something so I wrote it "can't be faster" not "it's slower".
The following sentences tried to explain why zero cost makes it faster by virtue of the alternative incurring a cost.
On the other hand, the reverse convertion, if(result == ERROR) { throw something; }, requires a conditional branch.
We are really getting into stupidly low level things now..
It was you who started talking about low-level and mostly irrelevant (for most code, at least) things like the speed of exceptions, so you shouldn't be very surprised if people follow the same route in their arguments.
SFML would do such condition if it were to throw instead, stbi, ifstream, FILE * and GL all return error codes not throw.
But SFML has to if if it wants to return an error code too.
And SFML already has many branches, even in Texture.cpp itself, that are false for good data.
If that's the case, could you show they're not necessary, then file bugs or pull requests to remove them? But if they're part of the logic, then they're part of the logic.
If you disagree, that's fine. But it'd be kind of you to point out the parts I got wrong and why I am wrong instead of using sophistry.
-
Error handling in SFML is hardly ever time-critical, and even if it were, other points such as user-friendlyness are certainly more relevant. I think we should focus again on design aspects rather than micro-optimization.
I mostly just said the whole "image loading" thing as a way of highlighting my attitude towards design and programming, namely I prefer very discrete responsibilities in code.
As mentioned, a separate loader would tear apart the implementation in many cases, which is not what I call well-distributed responsibilities.
If you look at the implementation of Texture.loadFromFile, it just calls through to sf::Image.
Yes, but look at other resource classes, for example sf::Font. Extracting loadFromFile() leads to another class that needs access to the same low-level library (freetype in this case). It is likely that both Font and FontLoader require common functionality, which leads to either further types that transport the information, or the break of encapsulation through friendship, or both. There is additional code necessary to pass information from one object to the other, i.e. there is more code to test and maintain.
On the other hand, I see no advantage of separating the classes. Please name an actual one (not just from a theoretical OOD perspective) that would justify the big disadvantages, namely more complex API and implementation.
-
I only refuted the part about "breaking in presence of foreign frames", the rest may or may not be true, my argument is not concerned about that.
Me, GCC for Windows page, MinGw 64 wiki page and TDM quirks page all say that DW2 breaks them and SJLJ doesn't, you said it's other way around, you got this part wrong too.
But SFML has to if if it wants to return an error code too.
But you were complaining that throwing after checking error code is inefficient so that's why it should always throw, while SFML would do exactly same thing in a function one level lower on call stack -> check one of error codes and instead of returning false throw. Something at some point has to branch on error code from C to throw.
The following sentences tried to explain why zero cost makes it faster by virtue of the alternative incurring a cost.
Reread the point, it was about code->throw being nicer conversion to write than throw->code. Here is the point pasted so that you don't forget to actually read it when you start replying, like you forgot to read all linked pages apparently.
3.The fuss of converting error code to exception is way way lesser than fuss of converting exception into error code:
This may or may not be true, but honestly, I don't understand how this relates to what I said.
You said basically: "SJLJ sucks beacuse DW2 crashes loudly and SJLJ does stupid things silently and corrupts state" while GCC page says it's perfectly normal for people to do exactly that using SJLJ and that's why Windows is stuck with this slower model and TDM quotes better interoperability as reason for using SJLJ by default.
If that's the case, could you show they're not necessary, then file bugs or pull requests to remove them? But if they're part of the logic, then they're part of the logic.
These lines make no sense unless you want to pass negative offset of left corner and have it ignored silently, two checks that have yet to be true in my code ever(I guarantee I never in my life tried to load image with left or top being negative):
https://github.com/SFML/SFML/blob/master/src/SFML/Graphics/Texture.cpp#L213
They are probably there for idiots so it doesn't crash in release.
It was you who started talking about low-level and mostly irrelevant (for most code, at least) things like the speed of exceptions, so you shouldn't be very surprised if people follow the same route in their arguments.
No, it was TDM and GCC pages that say SJLJ is always slow and that DW2 is even slower when exceptions happen. You are getting into CPU instructions level issues and claiming that branching on error code before throw is slow except you didn't provide any evidence for your claim whatsoever.
But it'd be kind of you to point out the parts I got wrong and why I am wrong instead of using sophistry.
You are using sophistry and spewing bullshit, you first said that DW2 and SJLJ are other way around with regard to foreign frames which is debunked by TDM and GCC page.
Then you claim that SJLJ actually is bad for not crashing because it breaks the foreign clean up, which is debunked by GCC page and called common practice.
Then you claim that checking single error code that is almost never true is a costly branch, while SFML does branches that are almost never false already.
How linking to official GNU, MinGW and TDM pages is sophistry is really beyond me.
You have yet to provide a single link, documentation page or whatever to back up anything you said and you are calling me using official sources "sophistry".
IF as you said SJLJ craps on clean up of C foreign frames then go report the problem to Dragon so he can update TDM page AND find more recent GCC on Windows page that says that and then come back to me because I'm completely uninterested in "feelings" such as:
This may or may not be true
may or may not be true
aimed at GCC or TDM page without any backing information whatsoever coming from someone who just two posts above this one was completely wrong about which exception model actually breaks foreign frames and now claims that both actually do and no one at GCC, MinGW nor Dragon noticed.
break of encapsulation through friendship
Does it really? BS says It doesn't: http://www.stroustrup.com/bs_faq2.html#friend
-
FRex and Silvah: Can we now return to a normal discussion and leave your personal feelings completely aside? You can still fight each other on your own private messages, but this near childish behavior and calling names won't help you, nor us, nor the discussion. Thank you!
Also a design discussion is about showing different opinions, possible dangers or problems as well as trying to find a general solution for the greater good. Thus it's important to give your opinion, but when doing so please keep in mind to not claim your opinion as "the truth" and "the right way" to go, thus kind of provoking fights rather than discussions (even implicitly!).
Side notes such as "everything-is-throwing-cause-modern-cpp-is-one-size-fits-all" are completely unnecessary and even though not intended can be seen as intimidating.
Thus keep it objective, fact based and try to hold your personal feelings back, that way we can actually get a good discussion going. :)
Now back to the topic
There have been a few arguments regarding C or binding usage. Those are valid points one has to think about, but SFML is a C++ library and we should be using C++'s full power and go with modern "patterns". Constantly thinking about how it would work in C or how C functions handle X or how bindings would work with it, doesn't yield many useful results to a C++ based discussion. Also keep in mind that a lot of other C++ features don't work with C and other bindings and yet we have a working CSFML and quite a few bindings to SFML.
I think the first iteration of the discussion should be about the C++ library SFML and only in a second or third instance one could think about what it means for all the existing and future bindings. So let us have a C++ rather than a C discussion! :)
Given that this discussion started from Tank's GDD article, it's interesting that nobody has really thought about what Tank actually said in the article. If we'd assume loadFromFile throws an exception, how can we prevent stepping into it? Do we have to implement our own exists() function, should SFML provide such a facility or do we ignore what Tank wrote and let everyone walk blindly into the exception?
Of course there are also other ways loadFromFile could fail, do we need facilities for that as well or do we just cover the most common one?
Maybe this goes a bit into the same direction as MorleyDev pointed out. The loadFromFile does essentially "too much".
Since some people rather want to stick with booleans, how would one fix the problem of not knowing what the error actually was about? Do you consider a enum with the different error and error codes to be a better design then?
And as a final note: Arguing about performance especially for such low-level functionalists seems rather ridiculous. Unless there are real benchmarks with function usage similar to how SFML would use exceptions/uses booleans this subject shouldn't really matter in such a discussion.
I'll also leave this (http://stackoverflow.com/questions/99683/which-and-why-do-you-prefer-exceptions-or-return-codes) here. It's not like we're the first one having such discussions. :D
-
Actually its more than 2 things that are done by the loadFromFile methods: access of the file system to get the file size, allocating memory for encoded data, loading the data, identify the data format and the size after decoding, allocating memory for decoded data, decoding, freeing memory from encoded data, using the data(upload to graphics card, creating glyphs and so on).
SFML hides memory allocation already for many things and I think that should be kept that way in the future, so that part can be ignored. Then there remain just loading, often decoding and using; and I think what can be easily done with a function needs no loader or decoder objects just to satisfy some OO extremists.
If those 3 responsibilities get split it would nicely separate concerns and the resource classes would stop needing to know how to access all kinds of other libraries. It could look like these (assuming its with exceptions):
sf::Buffer buf=sf::LoadFile("picture.png");
sf::Image img=sf::DecodeImage(buf);
sf::Texture tex(img);
sf::Texture tex(sf::DecodeImage(sf::LoadFile("picture.png")));
Then people could easily replace parts, for example:
FancyData data=fancyBufferedZipFileSystem.load("picture.png");
sf::Texture tex(sf::DecodeImage(data.ptr(),data.size()));
std::unique_ptr<uint8_t[]> bitmap=MyRaytracer.renderImage(x,y);
sf::Texture tex(sf::Image(x,y,bitmap.get()));
sf::Buffer buf=sf::LoadFile("picture.newformat");
MyBitmap bm=MyNewDecoder(buf.ptr(),buf.size());
sf::Texture tex(sf::Image(bm.x,bm.y,bm.ptr));
Maybe even add another variant of the Buffer?
sf::MemMapBuffer buf("picture.png");
sf::Texture tex(sf::DecodeImage(buf));
Another weird example for demonstration:
sf::Http h("http:://www.example.com");
sf::Http::Response response=h.sendRequest(sf::Http::Request("downloads/myshader.vs"));
if(response.getStatus!=sf::Http::Response::Ok)
throw std::runtime_error("could not download vertex shader");
sf::Buffer vsbuf=response.getBody();
response=h.sendRequest(sf::Http::Request("downloads/myshader.fs"));
if(response.getStatus!=sf::Http::Response::Ok)
throw std::runtime_error("could not download fragment shader");
sf::Buffer fsbuf=response.getBody();
sf::Shader shader(vsbuf,fsbuf);
Without exceptions that would all get longer and possibly loose the return values for getting something useful, but I think you can imagine the code yourself. Providing versions with and without exceptions shouldn't be all that difficult.
-
Given that this discussion started from Tank's GDD article, it's interesting that nobody has really thought about what Tank actually said in the article. If we'd assume loadFromFile throws an exception, how can we prevent stepping into it? Do we have to implement our own exists() function, should SFML provide such a facility or do we ignore what Tank wrote and let everyone walk blindly into the exception?
Of course there are also other ways loadFromFile could fail, do we need facilities for that as well or do we just cover the most common one?
A lot of this didn't quite make sense to me, because having a non-throwing version of/option for the function (especially if the optional argument is an error struct) seems to be the obvious and complete answer to all of those questions. Are you assuming we won't have such a thing?
Maybe this goes a bit into the same direction as MorleyDev pointed out. The loadFromFile does essentially "too much".
Well, let's try to get a bit more specific, since encapsulating all the annoying details for you is a big plus of OOP in general and libraries like SFML in particular.
The first few specific ideas regarding how to split it up have essentially been rejected in this thread already for splitting the API and implementation without any non-theoretical gain, so I'll ignore those. wintertime's sf::Buffer idea at least seems like it might have a purpose.
Since it's already possible to load the sf::Image yourself and make the sf::Texture from that, instead of doing it all in a single sf::Texture.loadFromFile() call, I'll assume we don't really need to debate that split (no one's suggesting we *force* everyone to explicitly construct the Image themselves, right?).
So the question is whether splitting sf::Image into sf::Image and sf::Buffer gives any real benefits. First, though I know very little about this low-level stuff, I have a hard time believing that you can load a file's data into a buffer with absolute zero awareness of the file's format, in which case you'd be splitting the implementation again. And what would be the benefit of manipulating the buffer yourself? Most of the examples wintertime gives seem like they should already possible with loadFromMemory and/or loadFromStream, so it seems like the only potential gain is some options for manual memory management.
Maybe you could argue that all of these functions shouldn't be part of the sf::Texture class because of the abstract design principles people have been citing, but I'm pretty sure that if you really follow those principles you just end up putting all of sf::Texture's functions into sf::TextureManager, leaving sf::Texture as nothing but a wrapper for an OpenGL cache ID (and sf::Image a wrapper for a Uint8*, etc), and at that point the new sf::TextureManager is identical to the old sf::Texture but with a longer name.
-
@wintertime: it's an interesting design, for sure. However, apart from a few extra constructors and a new interface with sf::Buffer, it feels a lot like the current loadFromMemory methods.
Sure, with exception, you can know exactly which operation / object failed but it can also be done with different type of exception (e.g. sf::IOException, sf::FormatException, ...).
And without exception I feel the code will be bloated with if statement everywhere without a real advantage over the current model – even if the user know what failed he will have to suffer too much.
About exception VS return code, here are my quick thoughts.
With SFML 2 we have to use code like this:
sf::Resource x;
if (x.loadFromY(z)) { ... }
With exception we could directly load x in its constructor and therefore make things more readable (IMO).
sf::Resource x(z);
Where Resource is Image/Texture/Sound/......
(This was also mentioned by someone earlier:) If we have to load N resource, we have (usualy) only one try-catch whereas we would have N if statements.
-
A lot of this didn't quite make sense to me, because having a non-throwing version of/option for the function (especially if the optional argument is an error struct) seems to be the obvious and complete answer to all of those questions. Are you assuming we won't have such a thing?
Never mind that, I should've looked at it again. :D
The answer is simple: You can’t guarantee that between your checks and the actual call, the file will still be readable, because you don’t control it. Files live on a shared storage, a lot can happen in the meantime.
The first few specific ideas regarding how to split it up have essentially been rejected in this thread already for splitting the API and implementation without any non-theoretical gain, so I'll ignore those. wintertime's sf::Buffer idea at least seems like it might have a purpose.
The question we'll eventually get back to will be: Is that still "simple" as in "Simple and Fast Multimedia Library"? How many more intermediate classes can we add to gain only a small bit of "better" design?
Don't get me wrong I like the idea and iirc we had some similar suggestions a year or so back.
-
Yeah, that was the conclusion I came to in that post (same as you and Hiura it seems). By "might have a purpose" I merely meant that it wasn't quite as straightforward a case as the sf::TextureLoader class proposed earlier. That one obviously didn't add any useful functionality.
-
The idea is to make all resource classes simpler by cutting out the 6 loadFromMemory, loadFromFile and loadFromStream. The Buffer class would then also replace the weird self-defined stream classes and the file loading for all things would use it, which arguably would make it simpler.
DecodeImage would be a free function and the only one to know about the png, jpg and other libs and then Image needs only know about already decoded bitmaps. Maybe the other classes could also be simplified in that way.
-
FYI I never proposed a TextureLoader, I was going more the ImageLoader/Load file and Decode way at the Image level.
I really should of thought of that splitting down to File -> Buffer -> Image, kicking myself for not when it's exactly where my thinking was heading and then I just couldn't finish it in a way I liked. But that way you presented, I like. It looks very nice and, I think, would read very well and be conceptually easy to understand.
For "simplicity" one could perhaps create a facade onto that system that unifies it for beginners, and when they need more complex solutions "Well, this actually uses these under the hood so all you need to do..." is always a nice thing to be able to say.
A benefit I kept thinking of and never bloody mentioned for some reason (sorry, it's been a long day xD) with an extensible design would be the ability to easily plug-in ones own loaders alongside the built-in ones for loading custom file types. An exposed loading system would be much easier for a user to extend.
Loading for images is, I believe, determined in the underlying libraries used in SFML by the "header bytes" of the files that image types have to identify them. A unified decoder you can add your own logic to would be very useful for people who want support for whatever types SFML does not provide support for.
However, I'm well aware all of these proposals would call for big changes in the underlying code for SFML. It's possible to instead write your own loader and call loadFromMemory/loadFromStream means it has solutions in the current way of doing thing.
Since the current way still works and can be made to work with others fine, it's arguable that it's not enough of a benefit to justify what could be a dramatic rewrite of the system.
But on the other other hand, writing a PhysFS File loader makes more sense to me than writing a PhysFS Stream as something to just drop into your code as a replacement for sf::LoadFile and have it just work...feels much simpler than having to go in, replace loadFromFile with loadFromStream and give loadFromStream the new stream you have to create.
-
I don't really see the problem with loadFromMemory/loadFromStream. I never felt they were "straying from the way SFML wants you to do these things." The whole reason they exist is to cover the odd cases where we want to get the data our own way; using them in those cases is exactly what SFML would want (if it was sentient).
The idea is to make all resource classes simpler by cutting out the 6 loadFromMemory, loadFromFile and loadFromStream. The Buffer class would then also replace the weird self-defined stream classes and the file loading for all things would use it, which arguably would make it simpler.
Wouldn't this be another case of splitting/duplication huge swaths of implementation details?
I guess you could argue the API is a bit simpler if you just use three overloads of sf::Texture instead of the three loadFromX methods, but that seems like hiding something from the end user you really shouldn't be hiding, and it'd probably be a little more confusing than having the explicit function names.
-
The idea is to make all resource classes simpler by cutting out the 6 loadFromMemory, loadFromFile and loadFromStream.
But this "simpler" is very local, the usage is massively more complicated. Three or more classes to load an image is definitely not the solution. The 7z API is built like that: You have to work with buffers and low-level algorithms and need to know a lot about the non-documented implementation. That effectively makes 7z/LZMA the worst C++ API I have ever seen. If we go in that direction, we should still provide a high-level way to load an image in one function call. Most people don't care about decoding, buffers, file formats or anything like that.
The Buffer class would then also replace the weird self-defined stream classes and the file loading for all things would use it, which arguably would make it simpler.
But using fixed buffers, you lose the whole advantage of sf::InputStream, namely the streaming itself. Now it is possible to load music continuously in chunks from an archive, how would this work with the buffer API?
One possibility would be to adapt sf::InputStream to work with sf::Buffer instead of (void*, size_t) pairs. But then you still have the stream class, and everything is more complicated.
-
We are getting rather distracted with the Image part of this, streams like for sound do add some complexity...hmm. I find it funny this was literally an aside-comment of mine, and it's turned into a debate alongside the exception one >.< Makes me feel guilty.
I definitely agree if any library did go this way, it would be useful to provide a facade to hide the complex innards that are unneeded for the basic use-cases. But I would expect that facade to need to be be nothing more than a few line long negotiator between the existing classes/functions.
I don't really see the problem with loadFromMemory/loadFromStream. I never felt they were "straying from the way SFML wants you to do these things."
Yeah, I didn't like my wording of that so I removed it from the post right after I clicked "post" xD Guess you saw the old version >.<
-
Here's another thought.
If you REALLY want lower level control than SFML gives you, shouldn't you be looking at the libraries that SFML is built on? OpenGL, OpenAL, stb_image, etc? If you want to muck around with stuff as low-level as manual memory management or adding your own decoders, it seems like you'd have to work with those libraries or reimplement parts of those libraries or better yet, submit your new decoders to those libraries (and use your own custom build for a little while until the patch makes it downstream).
I guess this doesn't apply for archive files, but for those you probably want to link some other library (doesn't 7zip allow that?), write a function to use it and then just use loadFromMemory. Is there anything SFML could do to make that any simpler, other than adding some loadFromArchive functions?
-
Correct me if I'm wrong, but the Music class seems the only thing that is really profiting from streaming. All other things including short sounds should be small enough they can easily be loaded at once and you can't use half an Image or half of a Font file or half of a Shader.
-
I'm pretty sure InputStream is never used unless you call loadFromStream, so I don't really see your point there.
-
(…)
Could you calm down, please? I really didn't mean to offend you.
I am familiar with the sources you've linked to. I also agree with them for the most part, i.e. regarding all points except the one about throwing across foreign frames. And even then, assuming the foreign code you're throwing across either doesn't need much cleanup or is specifically written to handle sudden changes of what piece of code is executed right now (given the amount of backwards compatibility hacks Windows has, I wouldn't be surprised if Windows API callbacks belong to the latter category), they're also right.
My claim refers to the situation where the foreign code doesn't satisfy this requirement. If you really want a credible source for that, you can download GCC sources and read libgcc/unwind-sjlj.c and see how it behaves, and that its behavior is more or less in line with what I described, that is, that it blindly longjmps to the most recently registered handler with little regard to anything.
You can even write a sample program and see the behavior for yourself. Say, a C DLL compiled by MSVC that's called from GCC/sljl-generated C++ code and that calls a callback and right after it performs some "cleanup" code (a printf is more than enough for a sample program), where the called back function is that C++ code and it throws an exception.
claiming that branching on error code before throw is slow except you didn't provide any evidence for your claim whatsoever
All branches can be slow, for the very same reason, i.e. branch mispredictions. You probably can find credible evidence in Intel or AMD optimization manuals. For example, the Intel optimization manual from April 2012 says, in 3.4.1, "Eliminate branches whenever possible" at the beginning of a chapter that contains a lot of information about why branches are the way they are.
At any rate, isn't "code that doesn't exist runs faster than code that does" a common sense?
Anyway, can we just agree to disagree and carry on?
FRex and Silvah: Can we now return to a normal discussion and leave your personal feelings completely aside? You can still fight each other on your own private messages, but this near childish behavior and calling names won't help you, nor us, nor the discussion. Thank you!
I don't really see any name-calling. Yes, people can and do get passionate about some issues, but this merely shows they care. Perhaps they care a bit too much, but that's real-world people for you.
Moreover, disagreement is an important part of discussion, after all, why discuss at all if we all already agree?
-
Heys guys,
at first: nice discussion. Like someone else said I'm happy to see that people around SFML are highly interested in core issues. It's one of the things I really love about SFML and its community.
Enough picnic, back to the serious side of life. :o
Exceptions, error codes or both: I'm with eXpl0it3r. SFML is a C++ library, so C++ techniques, practices and workflows should apply. If that requires CSFML to try/catch exceptions and forward them to the target-language's error mechanisms, then that's the way to go in my opinion.
Yes, this might be annoying for the binding writer (repeating code) and add another thin layer between the call and the execution in the library, but if that's required to reach a proper C++ way of doing things in a C++ library, well, then it's the cost. And unless someone can prove with a profiler and good tests that performance is really lacking, then I'm sure there are options that can be tried out.
Just look at how the bindings themselves are doing it: They change function names, implement event loops on top of SFML etc., just to make the binding fit to the target language. This is good, as nothing is worse than code that feels alien in a language (and does not make use of its security features especially!).
If your only argument against exceptions is that bindings are harder to do, then I guess that it's not valid. If SFML uses exceptions, then CSFML won't, because it's C. Since bindings are already using CSFML, I don't see any problems here. At least that's what my impression is, correct me if I'm wrong (it has been some time since I had been adjusting the Python binding).
It's not like exceptions are only a fancy way of doing error reporting. They greatly enhance security.
Splitting loading/saving routines: I like this. It's indeed an OOD thing mostly, but personally I think it's logical.
The biggest argument I can bring to the table (or: which is high priority for me) is modularity and security.
If sf::Image, for example, only contains the data, then the external loading mechanisms have to use the image's public interface, which makes sf::Image more robust due to encapsulation. Side effects are reduced, and transparency at the user increased.
Enhancing the image loading is easy as well: Just add another function/class that uses sf::Image. And this will not look alien/confusing to the user. Just compare these:
sf::Image image;
// First version, using internal loading routines:
image.loadFromFile( "foo.png" );
image = PerlinNoiseImage::generate();
// Second version, using external loading routines:
image = sf::ImageAdapter::load( "foo.png" );
image = PerlinNoiseImage::generate();
The second version is more intuitive for extensions, because if you are using an extension or not, the handling is the same (in fact SFML's "built in" loading routines would behave like an extension as well).
-
Splitting loading/saving routines: I like this. It's indeed an OOD thing mostly, but personally I think it's logical.
I won't say that's an illogical way of doing things, but... this second version seems just to add some factory classes/methods – which can already be done on the user site. In fact, it removes some flexibility in the user code style.
When Laurent removed most of the inheritance in SFML it was, among other things, to let the user decide if he wanted to use or not use inheritance for his custom transformable / drawable entities.
It seems the question is again on the table:
Should SFML force the user to use exception? I would say «yes», see Tank's and eXpl0it3r's arguments to which I agree. Also, there were some suggestions to use overloads and error codes so we can have both exception and "errno".
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).
-
I don't mind SFML supporting exceptions, as already said "it is the C++ way". C++ code shouldn't really have to care about what other languages support and don't support. If they want to interact with C++ code, they should be the ones taking the initiative and not the other way around.
As for the inheritance, I can say personally that whenever I design systems I tend to end up with really really deep inheritance hierarchies. Is this what I wanted? Not really. But until I figure out a better way of doing things, I'll just have to put up with myself :P. Just as a side note, deep inheritance hierarchies used to be the "in thing" back in the day when OOP theory showed up on the market, as we move further into the future, less and less people support having too many useless layers of abstraction. However, until that is what they teach new programmers learning OOP, you will probably keep seeing deep hierarchies (thanks Java ::)). A guideline in the design of C++ standard libraries is also to keep hierarchies shallow, probably also to keep compile time and overhead down since we don't want really fat classes polluting our objects when we use containers for example.
Maybe SFML should try to adopt some (but not all) of the guidelines at: http://isocpp.org/std/library-design-guidelines
Not only would it make SFML more modern, but as hard as it is to believe, I think it would actually make the library easier to learn if you are already an established C++ programmer. As for the non-established C++ programmers.... that is a story for another time :).
-
However, until that is what they teach new programmers learning OOP, you will probably keep seeing deep hierarchies (thanks Java ::)).
Coincidentally I just had Java loving doctor teach me OOP last year and one of the rules to pass it was to have, in final assignment project, at least 15 classes in a single rooted, single inherited hierarchy with at least 3 inheritance levels and to never write any free functions(writing them Java way if you really need them). :P
-
break of encapsulation through friendship
Does it really? BS says It doesn't: http://www.stroustrup.com/bs_faq2.html#friend
friend is better than making the members public, but worse than using the public interface. It still creates a cyclic dependency between two classes: The class granting friendship must know the name of the accessing class, and the latter must know the former's definition. This can be partially mitigated by an intermediate access class, but that again makes code more complex.
Correct me if I'm wrong, but the Music class seems the only thing that is really profiting from streaming.
The other classes benefit as well, especially if loading is slow. Consider cases like an encrypted and compressed file archive, resources that are received via network, and so on. When you have to wait until the whole resource is ready before processing it further, you may experience a significant slowdown.
Streaming is definitely something we have to take into account when designing a new API.
The biggest argument I can bring to the table (or: which is high priority for me) is modularity and security.
If sf::Image, for example, only contains the data, then the external loading mechanisms have to use the image's public interface, which makes sf::Image more robust due to encapsulation. Side effects are reduced, and transparency at the user increased.
As I already asked MorleyDev earlier, could you be more concrete here? It sounds all very theoretical. Sorry for nitpicking, but if the API should be changed to something possibly more complex, I find it very important to have a clear view about the concrete, actual advantages rather than nice OOD terms.
How is sf::Image more robust? Since there will be more code with two classes, more maintenance and testing is required. On the other hand, a separate class doesn't simplify the implementation anywhere.
What side effects do you mean, and why is the code more transparent for the user?
You proposed
image = sf::ImageAdapter::load( "foo.png" );
what about
image = sf::Image::load( "foo.png" );
? It would not tear the implementation needlessly apart, yet provide a way to have uniform factories. And it would be simpler to use since no separate class is involved.
By the way, these factory functions require C++11 move semantics -- recommended for all resources, mandatory for non-copyable ones. As a consequence, Laurent would have to drop C++03 support in SFML 3. If he won't, then we can directly forget about the factories ;)
-
Doesn't friend just require forward declaration of class that is giving access?
And it's using members because it knows what it's doing, so it's as a class was now split into two parts, one core that touches members and other that touches members AND files, so core class doesn't know what files are but it remains encapsulated so user can't break it, because he is not friend.
-
Doesn't friend just require forward declaration of class that is giving access?
Assuming you mean "class that is accessing": yes, that's why I said "must know the name".
The implications of friend are pretty obvious: One class depends on implementation details of another one. Thus, when you change the implementation, you have to adapt code outside the class. That's breach of encapsulation (by definition), however in a controlled way. I'm not saying there are no appropriate use cases for friend, but it should be used with care, and avoided when possible.
-
In my experience, the most common solution to situations where a deep inheritance tree or "friends" are starting to form is usually that if X needs to know about an underlying part of Y, then that is not truly an underlying part of Y but a separate Z waiting to be extracted.
Usually if you need friend, it's because there's a third class sitting in there somewhere, a dependency to extract that can marry the two. Extract it, make it it's own beast, and then inject it where needed.
But this kind of thing is much easier to do in languages with good refactoring toolage. C++ is not one of those languages, the available refactoring tools are limited and rather primitive compared to a lot of other languages. It's why I'm eagerly watching JetBrain's upcoming C++ IDE. They provide some of the best tools for developing in Java and C#, and to add C++ to that list? Gives me goosebumps.
The logic goes that the less an individual class/function does by itself whilst still fulfilling the duty given to it, the better because it means that each class/function is easier for the programmer/user to understand, to comprehend and to use effectively and efficiently, and so by reducing the complexity the possible mistakes and bugs in any given part of the system are reduced.
The more something does, the more complex it becomes, the more difficult it is to grasp it conceptually in it's entirety at any given moment. Quite simply, the more something can do the more you have to remember could be done to it and with it, the more difficult it is to use and maintain.
You have to remember more ways it can be manipulated and consider when those manipulations could happen and what their effects would be, and it makes coding in it more difficult, because you have to remember more ways it can be manipulated and how that could change the internals of the class and how the externals interact with it. It's more of a cognitive burden.
It also helps that having many small tools often brings greater flexibility by allowing for the composition of those small tools to achieve a wider variety of results with less effort.
-
In my experience, the most common solution to situations where a deep inheritance tree or "friends" are starting to form is usually that if X needs to know about an underlying part of Y, then that is not truly an underlying part of Y but a separate Z waiting to be extracted.
But it is not always appropriate to make Z part of the public API. Often, it is a tiny implementation detail, hardly worth outsourcing. Extracting the functionality in its own module may add flexibility, but it may as well decrease it, because the implementation is bound to the existing modules and loses the freedom to operate across similar, but now separated functionality. As long as one class is solely responsible for a task, it may exploit its complete knowledge for simplifications or optimizations -- when you try the same for several classes, each of them requires a big interface and a lot of boilerplate code for their interaction.
But this kind of thing is much easier to do in languages with good refactoring toolage. C++ is not one of those languages, the available refactoring tools and are primitive.
The possibilities are indeed limited, but do you know Visual Assist X for example? What level of refactoring do you exactly have in mind? I wouldn't expect tools for C++ that are able to perform high-level refactoring tasks (except for simple Java-style cases), simply because it's too difficult to implement them. Just look at overloading resolution, templates, name resolution and ADL -- not even compilers handle all those rules 100% correctly.
The logic goes that the less an individual class/function does by itself whilst still fulfilling the duty given to it, the better because it means that each class/function is easier for the programmer/user to understand, to comprehend and to use effectively and efficiently.
The less something does, the worse because there is more boilerplate code, more communication overhead, which in turn requires more development, maintenance, testing and debugging time.
The more something does, the more complex it becomes, the more difficult it is to grasp it conceptually in it's entirety at any given moment. Quite simply, the more something can do the more you have to remember could be done to it and with it, the more difficult it is to use and maintain.
The less something does, the more trivial it becomes, the more difficult it is to grasp the high-level concept in its entirety at any given moment. Quite simply, the less something can do, the more different classes/functions you have to remember and with it, the more difficult the project is to use and maintain.
Just to show you the other side, the truth is somewhere in the middle. But seriously, we need a less idealistic and more pragmatic view. There's always a tradeoff, and you should keep in mind that SFML aims to be simple, which conflicts with multiplying the number of classes to perform ordinary tasks.
I think we are meanwhile lost a bit in details. Maybe let's go back and ask ourselves how a simple-to-use yet powerful API would look like -- from user perspective, also taking into account beginners. And in particular, what is wrong with the current design (so far, the opinion seems to favor exceptions over bool values, anything else?).
-
Oh yes, it's always a trade-off that calls for pragmatism. I like and encountered most the phrase "Diminishing Returns" to describe it. Stop when the returns start to become diminishing, and continue again the next time you need to go to that code and the returns have become undiminishing again. It'd say it's more something it's good to strive towards.
But for a library I guess a degree more up-front design must be done than other projects which can limit the ability to be iterative.
For tools, Visual Assist X is a good start. I use it, it's good for exploring but the actual code modification tools are still a little limited. But it's C++, and comes with the territory. Still gonna miss the one-button "Extract Class" option xD
The solution to things that shouldn't be 'public api' but are by nature of the language..., well the cowboy in me says if it's not documented, don't count on it continuing to exist. But that's probably not a good attitude, hence why I call it "Cowboy Me". The most common way I see usually involves a detail namespace in C++. SFML seems to take the "put those internals into the CPP files and compile them in" approach.
*sigh* I'm going more into general design approaches now because this is something I just find generally interesting, and straying away from the main SFML topic.
-
Simply put, this is what development of a small indie game would look like if you ask me (horrible diagram ;)):
General idea of a game
||
\/
More concrete idea of the game (basic design etc.)
||
\/
------------> Write code
| ||
| \/
| Done? -- yes ------------------> Test
| ||
| no
| ||
| \/
|- no -- Can't implement something?
| ||
| yes
| ||
| \/
| Look for library to get things done
| ||
| \/
| Use library facilities in code <--------------------------
| || | |
| \/ | |
-- yes -- Done using library? | |
|| | |
no | |
|| | |
\/ | |
Don't know how to do something? -- yes --> read doc |
|| |
no -------------------------------------------
As strange as it sounds, the goal is to minimize the time spent using the library. The less time it takes to get something done with SFML the more time can be spent doing application specific coding, which if you ask me is the more rewarding part. Because of that, it shouldn't be offending if someone says they used "just" that one feature of SFML and they e.g. had an image loaded in 2 lines. If you want to write a conceptually "clean" library then you are free to do it, but it shouldn't be at the cost of the productivity of developers such as those from the horrible diagram above. Interfaces should be designed to reduce the time spent in both the "use library facilities in code" and "read doc" loops because it means that the overall time spent using the library will be minimized as well.
As Nexus already said, adding extra classes is a mental burden. I don't know anyone who finds an interface simpler the more classes it has... and classes can be anything in this case, including pure interfaces. The typical SFML user should have to create only that which they need and be able to do everything with that which they need. If you have a pure Image class, what could it do on it's own? It would have no information to start with, so an ImageLoader class would be nothing but pure boilerplate. You say you want to be able to customize the loading of resources? The C++ standard library already showed us how this is done: Traits (http://accu.org/index.php/journals/442). This is one of the things that makes C++ unique to other languages. The reason learning how to use the STL is that simple can be partially attributed to the usage of traits. For the newcomer to the language, they can get what they want done without caring one bit about them. For the advanced users who want to use memory pools, what do they do? They provide a specialized allocator and the job is done as well, without any noticeable difference for the beginners. The interface is not changed at all. I've become very keen on exploring traits more, especially with C++11 making more use of them than ever before. It would be interesting to see SFML make use of them in some way as well.
I think SFML evolution should be driven by what makes it more extensible while keeping it as simple as possible and minimizing boilerplate. The success of libraries and what drives people towards them is not how they are designed, but rather how fast and easily you can get things done with them.
I have great fun programming my own games and seeing how others have fun programming their own game as well. I know how frustrating it is when you are stuck on something that you know has to be done but has nothing to do with your original idea. And reading forum posts everyday, it seems that there are many common tasks that can still be simplified if you ask me. This is also the reason why I wrote a network library that takes care of object synchronization across multiple hosts. I had to do it in every single project I worked on, and judging by the network related posts I see, that is also what everyone else has to do all the time. This is an example of boilerplate, and it needs to go.
-
My point way-back-when was simply that 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. People disagree, cool, I can accept that and understand why. I defend my opinion as reasonable but can understand the others and see that they too are reasonable.
Plus, I also acknowledge that the significant rewrite it would take would be presently more effort than gain. I feel that has caused some confusion, I may have not made it clear I've been having a theoretical conversation that I acknowledge has some practical limitations with the current system that would most likely make such a redesign more effort than it's worth.
Although with you're diagram...Well, I may be an odd duck since my first step when confused with a library usually isn't to browse the documentation but to browse the autocomplete and even the headers for something that looks useful and see if I can use that. If they fail, documentation is the next port of call before 'random googling in the hope someone posted something on a forum somewhere'.
EDIT: And I was in the middle of rewriting this, so...I guess I'll move it here. Exceptions!
I am being won over by the "two-interface" approach, main concern would be a lot of boilerplate code would get into SFML though and it'd be annoying to maintain.
I'm guessing there'd one implementation that outs the error, and one that passes through to that outed error and throws a different exception based on the error? Or the other way around? Actually I'd probably prefer the other way around, it lets SFML at least keep the advantages of exceptions internally even if that doesn't get exposed to the user who doesn't want them.
Either way, it seems like it could be a lot of boilerplate to do that for every function that could throw an exception.
-
Maybe it's just me, but I see exceptions as things that can happen but really shouldn't.
The most disgusting code I have seen that uses exceptions was written in Java by a person I was studying with. They claimed that they didn't do bounds checking when iterating through a multidimensional array because they would use the exceptions that were thrown to navigate their broken loops. This is a case of "exceptions becoming the norm".
We mustn't forget, the reason they are called exceptions is because they present an exceptional case. One could argue that they are bound to be thrown at some point, but if you ask me, we should focus on preventing them from being thrown in the first place. An example: if you are scared of exceptions being thrown when loading a resource, consider why it would be thrown: if it doesn't exist as an example. How can you prevent it from throwing in this case? By making sure that the resource exists before you load it, through some other facilities.
You might think I use exceptions as a glorified fatal error mechanism, and you would be correct. If you are already ready to handle one that comes along, you could have spent that effort trying your best to prevent it before trying to load in the first place. I would call this "making exceptional behaviour into standard behaviour". If you want both cases to be viable and execution of the application to continue, do so in non-exception code. It makes it easier to understand that it is something that can happen and is nothing bad at all.
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 :P).
-
I'm not really into Java, but from an outside view it definitely looks like exceptions are tedious and overused in it. Especially cases when some standard library class throws some checked exception and you pretty much have to put a try/catch around it to rethrow something unchecked or modify the whole calltree. Or when every function in the call tree needs a try for cleanup, because stupid Java designers thought the only resource one would use was memory, but a garbage collector is not concerned with you for example not exhausting file handles while someday if you are lucky some finalizer runs.
In C++ this is a bit more sane, exceptions get used for really exceptional cases and if you do it right everything gets automatically cleaned up by RAII and destructors. A try block is a rare thing there, for when you know a second strategy to recover, to print some error message out in main or if you insist on wanting to play with insecure lowlevel things. But you can never fully prevent them, even if you check if a file is there beforehand there could theoretically be a bad sector on the HDD or the user could take out the CD or pull the USB connector or the server could stop answering or ...
Btw, I read some of the documentation of SFML recently and for some classes you get that long list of methods where only half of them are interesting to most people. There I really wished sometimes it was cut in two parts and I (or the library user just needing a little thing in that diagram) needed to concern myself only with one of those easily understood smaller parts.
-
"Exceptions are for the exceptional". The blog post in the thread that spawned this one makes a good statement: They are best used in the situations where you did everything right, but it still didn't work because of situations largely out of your control or what could be reasonably expected of you.
However, the question then becomes does SFML provide the means to perform that checking, to "do everything right"? And in what situations does the language sufficiently provide them?
Take the file example, should SFML provide the means to check if an image file exists? And it's decodable? Well the only good way to check the latter is to decode the image so it can easily be argued a non-decodable image is more or less an exceptional death scenario. But the former part, that a suggested file exists. Is that within SFML's scope? I'm not asking that because I think it isn't, this is more a question to promote discussion since I'm unsure.
And yeah, exceptions are often overused in Java. I can't stand checked exceptions, and the saner usage of exceptions are one of the reasons I much prefer Scala and C# for that kind of development.
As for Nexus' earlier suggestion about dropping C++03 support in SFML 3...yes please? It's been long enough since C++11 went gold. and SFML 2 is very usable by those constrained to C++03. I believe the need for new libraries to support C++03 has passed for the majority of situations (with the exceptions that I would avoid the threading part of the standard when possible until MinGW consistently supports it without needing experimental builds).
And as for following those library design guides, only part I really disagree with is the avoidance of inheritance and virtual functions. That rule is well suited for purely algorithmic libraries and code such as the STL, but I wouldn't say it applies for something such as SFML.
-
Maybe SFML should try to adopt some (but not all) of the guidelines at: http://isocpp.org/std/library-design-guidelines
That's an interesting reading, generally speaking – not sure if SFML should follow all the rules there, though.
Re: «one task equal one class» (to sum up extremely quickly the long – yet interesting – discussion above).
MorleyDev, your theoretical point of view is right, I agree. But I also agree with Nexus. Often (if not most of the time) a more practical approach is simpler. Here I think it's the case.
(Extreme) Case study: MVC. Recently I had to port a simulation software written a few years ago that use MVC a lot. My main task was to use SFML instead of a home-made OpenGL wrapper. Well, long story short, even if the code was relatively small (3k loc) I end up removing everything and starting from scratch because it was so complex to reason about the code due to its complex MVC design. I could reduce by a factor of nearly 3 the number of classes, and, more importantly, make it less buggy and more maintainable.
In the case of SFML, splitting the current Image, Sound, ResourceX, ... will introduce bugs (statically speaking, since we would edit a stable code), make it more complex and harder to reason about. So, if a proposed modifications give us more power, flexibility, ... Ok, why not. But currently loadFromMemory is enough for me and can be used to create any fancy facade easily.
I think it's one of those case where the theoretical approach is too academical.
Btw, I read some of the documentation of SFML recently and for some classes you get that long list of methods where only half of them are interesting to most people. There I really wished sometimes it was cut in two parts and I (or the library user just needing a little thing in that diagram) needed to concern myself only with one of those easily understood smaller parts.
Do you have concrete examples? :)
-
Maybe that would be one of the features that show up in SFML's feature list in the future... After all a Multimedia Library isn't really much if you can't access the filesystem... unless of course you hardcode all your resources into your application, but let's not give people ideas ;).
SFML already had issues with files not being loaded properly, or STBI causing mysterious crashes when trying to load a directory as an image, so some facility that prevents such things from happening would be a good addition if you ask me.
(Extreme) Case study: MVC. Recently I had to port a simulation software written a few years ago that use MVC a lot. My main task was to use SFML instead of a home-made OpenGL wrapper. Well, long story short, even if the code was relatively small (3k loc) I end up removing everything and starting from scratch because it was so complex to reason about the code due to its complex MVC design. I could reduce by a factor of nearly 3 the number of classes, and, more importantly, make it less buggy and more maintainable.
This sounds like the typical case of people (maybe fresh out of university or whatever) that are over eager to employ what they have learned. What is more important than understanding patterns is how and where to use them effectively. This is also the reason in one of my Software Technology exams we had to argue about why a pattern would be effective in a given sample of code and use case. I guess too many people forget about this too fast ::).
-
@binary1248, well it was an academic project to teach students how to make something – that's maybe why they chose this complex design. ::)
But that's the actual point. Let's not overthink the design. «Don’t be clever» said Standard Library Guidelines (http://isocpp.org/std/library-design-guidelines) – they also say «don’t be stupid», of course. ;-)
-
Btw, I read some of the documentation of SFML recently and for some classes you get that long list of methods where only half of them are interesting to most people. There I really wished sometimes it was cut in two parts and I (or the library user just needing a little thing in that diagram) needed to concern myself only with one of those easily understood smaller parts.
Personally, I found the small parts/shorter lists were always presented in the tutorials, and I generally check the documentation whenever I wanted to check the exact function signatures/behavior or look up the full range of features the class had. What classes are you thinking of exactly?
I'm guessing there'd one implementation that outs the error, and one that passes through to that outed error and throws a different exception based on the error? Or the other way around? Actually I'd probably prefer the other way around, it lets SFML at least keep the advantages of exceptions internally even if that doesn't get exposed to the user who doesn't want them.
Either way, it seems like it could be a lot of boilerplate to do that for every function that could throw an exception.
It seems like there should be a way to do this with only one implementation and a few if statements.
void Texture::loadFromFile(std::string filename, sf::Error* e = nullptr) {
...
if(e) {
e->set("Failed to load " + filename);
return;
} else {
throw sf::Error("Failed to load " + filename);
}
...
}
Maybe there's a nicer way of passing an error arg with a default that's easy to test against that I'm not thinking of.
Maybe that would be one of the features that show up in SFML's feature list in the future... After all a Multimedia Library isn't really much if you can't access the filesystem... unless of course you hardcode all your resources into your application, but let's not give people ideas ;).
Since there's going to be a Filesystem standard library in C++14 we probably don't need SFML to implement any of that for us.
-
Since there's going to be a Filesystem standard library in C++14 we probably don't need SFML to implement any of that for us.
What is there left to do then? At some point standard C++ libraries will be able to do everything... right (http://en.sfml-dev.org/forums/index.php?topic=12789.0)? ;)
-
lol, yeah, but I mean that one's already on its way and has a TS submitted and everything; by the time we reimplemented it in SFML everyone would have a C++14 compiler (if we believe Herb's presentations) and our work would be wasted. We may also get standard 2D graphics and audio libraries eventually but that's much farther down the line, if it's coming at all.
-
Btw, I read some of the documentation of SFML recently and for some classes you get that long list of methods where only half of them are interesting to most people. There I really wished sometimes it was cut in two parts and I (or the library user just needing a little thing in that diagram) needed to concern myself only with one of those easily understood smaller parts.
Do you have concrete examples? :)
For example I was reading through the documentation of the Drawable hierarchy and its maybe just a documentation thing and not a problem with the classes, but I was trying to find whats different and whats common and there some annotations in the short list for new methods, overwritten and slightly different, overwritten just to adapt to the underlying data but doing exactly the same and unchanged methods would have helped. Also the pages got a bit long with having to scroll up and down so splitting them in two may be helpful, maybe such that one could have a page with only the differences to the base class. Compared to other libraries SFML docs are nice already, but some improvements can always be had. :)
Then like was discussed above all those different create and load functions clutter up the resource classes, when there could be just 2 or 3 contructors mostly and people could just use a single generic Load for all of them and Decode could also be split off, though I see how that could disrupt current users a bit.
-
Honestly the function lists for Texture and Shape (since Drawable was trivial) don't look that long to me. I barely have to scroll at all unless I want to go down to the detailed descriptions, and for some reason don't like clicking on the function's name in that list. It might be hard to keep every single one of the functions in your head, but for those classes at least there are a lot of obvious groups that it's hardly overwhelming imo. Plus there's a tutorial specifically on drawables, so if you read that first the hierarchy ought to make a lot of sense.
Though I notice the position/rotation/scale/origin functions are all duplicated in Shape/Sprite/Text but not in VertexArray, and until you look around for a minute it's not obvious that Shape/Sprite/Text all inherit from Transformable, so *maybe* an improvement could be made there. I'm not sure quite what it would be though.
-
Maybe that would be one of the features that show up in SFML's feature list in the future... After all a Multimedia Library isn't really much if you can't access the filesystem... unless of course you hardcode all your resources into your application, but let's not give people ideas ;).
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.
Also the pages got a bit long with having to scroll up and down so splitting them in two may be helpful, maybe such that one could have a page with only the differences to the base class. Compared to other libraries SFML docs are nice already, but some improvements can always be had. :)
Luxury problems... :P
The documentation is limited by Doxygen. Considering its internal mess (I've had to modify the source code in several places), we can be happy it works so well :) I like the SFML documentation; it's really well done, you don't find that a lot for C++ libraries. And we should probably not design the API only for the sake of appearing nicer in the docs ;)
-
Come to think of it, I've heard people say/imply in multiple places that Doxygen docs are bad, but to be honest I've never seen good documentation that didn't use it (SFML's tutorials/docs are the clear gold standard out of everything I've read), and I've never seen anyone suggest an alternative. Of course good tutorials always have to be done by hand but they're a lot harder to screw up than comprehensive documentation. So are there alternatives?
-
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.
-
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.
-
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.« (http://en.sfml-dev.org/forums/index.php?topic=13240.msg92900#msg92900)
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 :P).
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.
-
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/ (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.
-
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.
-
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 (http://en.sfml-dev.org/forums/index.php?topic=13240.msg92876#msg92876) 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.
-
---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.
-
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/
-
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...
-
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 :D).
-
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 (http://en.sfml-dev.org/forums/index.php?topic=13240.msg92882#msg92882), 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) :)
-
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.
-
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.
-
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.
-
...
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 ;).
-
I really like this discussion, there's so many good points of view. I've my personal opinion for this specific topic but I could agree with the «opponent» idea in some other contexts. :D
Let me add two things. First, somewhat paraphrasing Laurent's regarding wintertime's last comment: I haven't looked at the source to check your claims but it's very nice to have some code review, thank you! Regarding your general comment (i.e. split the design) I disagree: it's an implementation detail/issue rather than a design issue. The API design should not care about how the implementation is driven to be efficient.
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.
[...]
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.
You're describing a Model-Controller(-View) pattern. In some specific case it's very useful, agreed. But here it would just mean breaking – or at least coming close to breaking – the encapsulation. The bottom line with such design is that the Model has some setData(...) method which does some validation check if needed and the Controller use that to set up the model object. Often the model and the controller are so interacted/intricate that having their two interfaces work together cost (to develop/use/...) more than having one class.
Instead it's better to reduce the interface «area» (and thus reduce the issue of interface communication) and mix both Model and Controller (sometimes also the view) to hide the data.
-
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?
I have to admit that I haven't checked the sources like wintertime did, but considering that Laurent has already split them up (I don't know when) tells me that it was indeed a positive move. Even if »only« for moving the bulk of unrelated loading code to another place (in my opinion pure design decisions are very important, because the result is what you, as a programmer, have to use all the time). The difference between my attitude and Laurent's is probably that I'd even split it if the external library only needed 2 lines of code instead of the walls it requires right now. ;-)
The loading functions are clearly specific to the respective resource classes, you can't have a generic loader in SFML.
I think loading something can be divided into a source, a driver and the data. Source can be the STL with its streams, the driver has to be custom, just like the data itself.
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.
If the separate class required full access, then it's in no way better than adding the code directly to the class itself. I'm sure that one can think of a nice interface to manipulate the image directly -- or even add a fitting ctor. I don't understand how the API is becoming inconsistent then. It's indeed a problem, not a big one, but real enough to make me think about it. ;-)
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?
No I've read it, and I've already given some examples + arguments why I think it's better. To me the API wouldn't be more complicated, it would feel more intuitive to me. That's why I'm fine with added »boilerplate« code too. And if SFML used unit tests, the debugging/maintenance part would be minimized too. ;-)
I think we've discussed it well enough. You've got your point of view on this which I partly even agree with. For me the current design is alright, there is definitely no show-stopper, just room to improve in my eyes, or let's say: for my taste.
No, I only use my own creations (previously the Thor logo) :)
Oh, sorry, I must have connected the logo to your name then. ;-)
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.
I fully agree, however before declaring something as being counter-productive, that has to be proven as well. There's always theory before practice, and experience alone does not justify ignoring new/different ideas. ;)
-
It's interesting that there is already an internal split that nobody noticed. A good example where implementation details are hidden from the user :)
I fully agree, however before declaring something as being counter-productive, that has to be proven as well.
That's not how it works in practice however; for something new to replace something existing, its advantage has to be proven. Even more, the advantage has to be big enough to justify throwing away existing approaches. That's also why interesting languages like Haskell and D struggle hard to get recognition, even if they do some things better. Of course changing the SFML API is a completely different scale, but it would still come with a lot of effort on implementation and user side. And in my opinion, this effort is worthwhile (mostly) iff the usability is improved. Now this shifts the problem to discussing about usability :P
Thanks for the elaboration, Tank. I think you're right, we know each other's opinion meanwhile ;)