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

Just further to Kevin's comment:

"We've tried to mitigate the problem of #2 by having a lot of mocks and stubs and nulls that are easy to plug in and make the object easy to manipulate in test. Since its easier to read a constructor than it is to trace what the call order is, I'd rather plug in mocks/stubs to the constructor than have to trace a call order to figure out if we've fulfilled our usage requirements."

This is false: "lot of mocks and stubs and nulls that are easy to plug in and make the object easy to manipulate in test". You define "easy" by how much effort it requires for someone like you guys (who have been at it for over a year) to modify the code. I define "easy" by how much effort a newcomer requires (with no prior knowledge of the code).

This is false: "Since its easier to read a constructor than it is to trace what the call order is". Constructors do not explicitly state what they're doing, unlike "set_id" or "set_event_sink". And if you did make a mistake somewhere and dereference a null shared_ptr, then the trace would be a call stack. Automatically generated and easier to read.