Code review comment for lp:~vanvugt/mir/surface-states

Alan,

612 + if (event_handler)
I thought of moving it too. But I prefer the cohesion of having code in the one and only place where it is used. And it's easier for the reader to comprehend when there is one fewer function name to look up and find. Should that logic ever be reused then definitely separate it, sure. It's not like that block of code is too big to fit on anyone's screen.

My first attempt (weeks ago) did work on the wire protocol as you say, and did not piggyback on Result. However there was a major problem with that -- Protobuf cannot tell you what kind of message you are receiving before you try to decode it. You just construct an object of every possible message type and try to parse the data for it. Half the time you will get an error (wrong message type) and waste CPU time in doing so. The simple and fast solution is to guarantee that incoming messages are of a type that you know in advance (Result). So you can parse them correctly first time. Protobuf won't log any errors and you don't waste CPU time.

The set_event_sink/casting stuff was a big challenge. I spent days (weeks actually) figuring out the class relationships and how to make them talk in a way that makes sense. Reworking it all could take days for me. Maybe simpler for you since you understand these classes. So I would appreciate merges proposed if you'd like it structured differently. I am rapidly running out of time and won't be around to do major reworks until June.