Code review comment for lp:~alan-griffiths/mir/surface-states-simplification

might be the first time there's been 2 rival reviews going on ;-)

again, seems the biggest point of contention is about 2 phase initialization and what has more or less coupling. maybe a new day will let me rephrase a bit better. :) Seems to me that (for example) in msh::Surface msh::Surface's notify_change function is coupled to having the ID, so whether you put it in the constructor or require a set_ function to be called, both depend on having an id to use notify_change() in a way. I'd rather come down on the side of putting it all dependencies in the constructor, because it allows us to see pretty quickly what a class needs, and gives us an easy-to-read signal about if a class is doing too much and needs to refactor. So, msh::Shell, might just be on the point of needing a refactor. (if its constructor is getting "too bloated" of course)

I know we had a debate similar to this one at an earlier point in the project, I hope we have another one next week :)