widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Tibor, sure, go ahead and merge.

> I looked at other examples f.e. here: http://bazaar.launchpad.net/~widelands-dev/widelands/request_supply_opt/view/head:/src/economy/supply.h#L94 and everywhere it is done this way. 

Ah, that is a philosophical question, really. Does local style trump global style? Should better paradigms be adapted peu-a-peu or in one big refactoring? There are many school of thoughts here. I think once you identified a better approach, you should start using it for new code and fix old code over time. Other people think that you should keep the old ways until you can change a full code base to new ways. 

So there are really two questions for you here: 

1) do you agree that passing mutable values by pointer is better than passing them by non-const reference? I find foo(&game) vs foo(game) a clear advantage for readability, so I'd say yes. This also follows the google style guide which I find a great reference of how and why to do things anyways. But you might disagree and I am in no position to enforce a 'true' widelands code style.

2) do you agree that new code should adopt new paradigms immediately at the cost of inconsistency in the code base or not?

As said, I answer yes to both questions, so I'd change the code to using a pointer. But it is entirely your call. I will not think any worse of you if you decide against changing the code :)
 
> Also note the 'const' at the end of line, does it not guarantee immutability of Game?

no, the const at the end of the line means that the object represented by 'this' cannot change, but does not say anything about the arguments of the method - they can change.


-- 
https://code.launchpad.net/~widelands-dev/widelands/request_supply_opt/+merge/280193
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/request_supply_opt.