I am uncertain about your proposition. I do not think that parsing the file with an XML parser is the best way to solve your problem. It is true that

your proposition is solving the issue but there are two things that I do not like:

1. It is now duplicating the "loading" code and also duplicating the way a Configuration File is loaded. Both Configuration::IsValidConfigFile() and Configuration::LoadFile() will have to be modified if a correction is required.
2. Each file are now parsed twice before loading. A single xml file is first parsed in Configuration::IsValidConfigFile() when validated here and then it is deleted/discarded and then a second time when it is actually loaded here.

When I have written Configuration::IsValidConfigFile() function, the purpose was to quickly discard files that were not Configuration Files. This was for performance reason. It we parse each file twice, I think it defies the performance purpose of first validating the file. I think it would be better to just load the file and see if the load is successful.

I think it would be a better idea to completely delete the function Configuration::IsValidConfigFile() and rethink how ConfigManager::Refresh() is implemented. For example, we could simply remove line 104 and let the line Configuration * config = Configuration::LoadFile(file_path, error); deal with actual file loading. If we try to load an invalid file, a NULL Configuration will be returned.

What do you think ?

Good!