widelands-dev team mailing list archive

[Teppo Mäenpää <tm@xxxxxx>]

Hello, Klaus,

Since you asked: Does the line like "return forester_cache[mi] = correct_val;" bring any performance benefits? I know it is valid and all, but the old way (separate assignment and returning) is more intuitive. Not everybody reading the source are professionals, especially C/C++ professionals. Apart from that, the changes improve. I have a bad habit of dropping magic numbers in.

About magical constants: The comparison (cache_entry > kInvalidForesterEntry) looks weird. It makes sense when one knows that the numeric value of the constant is -1. Would inequality comparison look better? No other negative values should be around.

Thanks for the review.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit.