Hey everyone! I've done some work on the engine in the last half year currently I am adding the .cpp files. Could you guys give some further tips? (Hint: I've redone the logging pls check it. Is it good?)
Sure, here's a few things I noticed:
//TODO: This MAY cause problems!
bool isOpen = (bool)(in);
See http://www.cplusplus.com/reference/fstream/ifstream/is_open/
Logger() {};
Logger(Logger const&){};
Logger& operator=(Logger const&){};
If your compiler supports C++11, consider using = default; or = delete; See http://en.wikipedia.org/wiki/C%2B%2B11#Explicitly_defaulted_and_deleted_special_member_functions
static Logger* Instance();
static void ResetInstance();
Consider making a templated Singleton class so you don't need to re-write this for every singleton you have. Personally, I try to stay away from them but sometimes they're appropriate (ie. a Logger class is quite appropriate).
#define LOG(Message_) Logger::Instance()->LogLine(static_cast<std::ostringstream&>(std::ostringstream().flush() << Message_).str());
Using a preprocessor to wrap any debug/log statements is a great idea, as you can easily have the #define statement do nothing for certain builds. However, consider utilizing the __FILE__ and __LINE__ macros to see where messages are popping up. This will help in the long run. (note that __LINE__ is an int).
inline std::string GetClockTime(void)
Consider making global functions as static functions in a class so you don't pollute the global namespace, and I don't seem to see any namespaces anywhere. Also, I assume this is C++ since you're using C++ tools, so remove (void) as it does nothing in C++ and only clutters things.
if (logNumber > 500)
500 is a magic number to me. Consider creating a constant that makes more sense to an external viewer.
if (!instance)
instance = new Logger();
At first, I was a believer of the "no braces for a single statement if statement" but unfortunately, it can be error prone (an error may only come up 1 in 1000 of these situations, but even that small chance is worth the coding style change), so consider always wrapping statements in { } braces.
instance = NULL;
Consider using nullptr. See http://en.wikipedia.org/wiki/C%2B%2B11#Null_pointer_constant
Logger* Logger::Instance()
{
if (!instance)
instance = new Logger();
return instance;
}
Consider returning a reference for a function if it's guaranteed that the item it returns will always exist. Only use a raw pointer if you cannot guarantee that the returned item will exist.
I also implemented my log/debug functions as functions that take a std::string as a parameter, but I probably should have just overloaded the << operator instead, so you may want to try that out as well.
I was just looking in TextureManager as well, and for your sanity, use typedefs or using = SomeC++11Type or using = SomePreC++11Type. I'm referring to this:
#ifdef CPP11_SUPPORT
std::unordered_map<std::string, SDL_Texture*> textures;
std::unordered_map<std::string, bool> errorShown;
#else
std::map<std::string, SDL_Texture*> textures;
std::map<std::string, bool> errorShown;
#endif
I hope this helps.