Review: Needs Fixing 1) Is strange, but makes sense. I do not fully understand why that is needed now and wasn't before, but meh.
2) Dangerous assumption. The speed of a dynamic_cast() depends hugely on many factors. String comparison is always slow. See http://stackoverflow.com/questions/9778145/how-fast-is-dynamic-cast for a more involved discussion. What I do not like about the new design is that we got rid of the enum {} we had and now use strings to identify our classes everywhere. This is slow, in many places where a switch() got replaced with an if() else if() at least by an order of magnitude or two. I think we have to reconsider again. Also the comments in the old enum reads like we did if (type >= BUILDING) in places which I do not like design wise (fragile!) but which broke if we ever did. The best design imho would be to bring back the old enum but as an enum class (http://www.cprogramming.com/c++11/c++11-nullptr-strongly-typed-enum-class.html) that does not allow >= comparison and replace type_name() in the c++ world through type() everywhere. We then need a function that takes a type and returns a string (for Lua and probably for logging output, not sure). -- https://code.launchpad.net/~widelands-dev/widelands/bug-1341081/+merge/227107 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1341081. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
