> "This concurrency model appears to be adding complexity to the system without any visible performance benefit." just screams "I think this is shit but I'm not

> "This concurrency model appears to be adding complexity to the system without any visible performance benefit." just screams "I think this is shit but I'm not gonna tell you more directly because we have a policy not to do that". Might be only me, maybe. After the knee-jerk "this is shit" reaction, the slightly deeper thought is probably along the lines of "There are better ways to do this that would simplify the code (reducing the chances of bugs, makes it easier to maintain) but it would basically require a complete rewrite". I think the challenge here is what you don't know is: Did the person writing the code just not see a simpler way? Did they iterate to this, and realize it's overly-complex but don't want to spend the time to redo it now? Do they think this is actually the best approach, and has some other merits that make it better than other approaches? How have others approached this in the past? I've definitely been in the situation where I had to review code like this, and I've definitely agonized over how to carefully phrase "I think this is shit" in a nicer way. If my review would really be "Sorry but I pretty much think we need to take a completely different approach and rewrite 95% of this" then I will typically have the conversation in person to discuss it, as it's easier to be clear I'm not criticizing the person and also avoid misunderstanding something (either direction). Sometimes it turns out I was missing something, but maybe there's still an intermediate way that reduces complexity but still addresses the problems the original did.