Remove duplicate code for sending events. - #234
Conversation
MartinDaCosta53
left a comment
There was a problem hiding this comment.
A clever idea to determine opc by using the offset in the table, but I think it makes the code more opaque. The more I look at possible implementations of compacting the producer service, the more I think that there is little gain and at the expense of clarity, which is the reverse to what I'd been looking for when I voiced my thoughts on the video call.
Therefore, I think that we should leave this code as it is.
|
I agree that the function findEventOpCode() is not clear. Should be documented better and should have unit tests. Or maybe rewritten with nested switches and returning the actual OPC_XXX constants. But that would introduce tedious duplications again. :( Is it just the findEventOpCode() function you don't like? Unfortunately it is crucial for deduplicating the send functions. I hope you liked the new send functions. The actual program memory gains were smaller than I hoped. I'll break down these changes to measure their impact better. |
|
Having pulled the file out to look at in isolation rather than one showing ommissions and additions from the previous version, it all becomes a lot less daunting. In fact, having looked at it over a couple of days, I rather like it. Just needs more comments to make it clear how and why it works.. I'd offer to do it but can't before the end of next week as I'm going away. |
Removed duplicate code after discussion with Martin.
Also replaced code that uses va_args as this seems to create larger codes.