So I've been reading parts of the book on my vacation and I've noticed a little issue. It's more a question of deign than an actual bug. The implementation of the resource holder makes use of assert, which is great and it's explained quite nice, but the issue is, that there's no way to prevent stepping into the assert, except by logical exclusion up on programming. As we probably all agree, we should write code that leaves as less doors for making mistakes as possible, thus having no way to check, whether a resource already exists from outside of the class, forces the user to either step into the assert or make unnecessary comments everywhere, which might get over read later on, to prevent the loading of two identical resources/with the same ID.
My conclusion is, that the resource holder class should have a exists() function to check the existence of resources, before loading them and praying it won't blow up in the assert. I mean one doesn't necessarily have to check the existence every time, but having no option to do so, seems rather dangerous/bad.
For examples:
TextureHolder textures;
if(!textures.exists(Textures::Airplane))
{
std::vector<std::pair<Textures::ID, std::string>> textures_assets = load_textures_info_from_file();
for(auto& asset : textures_assets)
{
if(!textures.exists(asset.first))
textures.load(asset.first, asset.second);
}
}
I know the other code looks prettier and one doesn't always have to check it if the logic allows it, but letting the user step into asserts because there's no way to check any kind of existence seems like a bad design.
Sure asserts are there for debugging, but you can't guarantee that all the possible scenarios will be played out, while running a debug build, thus releasing a game with possible bugs.
What do you guys think? Why didn't you implement such a function? :)
What about a separate forum for the book? :P
Assertions are nice for debugging and when designing a library, to make sure internal state are valid. But when there is an interface with the user code it's not the best tool. I would instead recommend exception or return code (for simpler case) instead.
I do not agree at all. Personally I separate between programming errors and exceptions. A programming error for example is this:
resource_manager.get( "non_existant" );
If there's no resource with the ID "non_existant", and the resource manager provides me with a function to check for that resource prior getting it, then I've used the class wrong. This is a perfect case for assert(). (By the way: make sure to run your stuff without NDEBUG/asserts enabled!)
If I do this, however:
resource_manager.load( "cool_stuff.png" );
and the file can't be loaded, then it's an exception, because I can't guarantee beforehand that at the time the file is going to be read, it exists, is readable etc. Ergo an exception should be thrown.
Lastly, my opinion on returning error codes: Just don't do it. C++ provides us with exceptions, so I don't see a reason for using error codes. They require me to check them every time (I can simply overlook them) and don't scream when I forget to do so. They also go down the whole callstack, which is awesome for error handling delegation.
By the way, bool values are also some kind of error codes. Personally I'd design, for example, sf::Image::loadFromFile() to be a void function. Laurent, would you still follow the same way of designing the functions or choose another if you'd start now? I ask because I changed my way of doing this too at some time. ;)