If I were on the receiving end of that feedback, I'd much rather have the reviewer write something like:
"Can you walk me through the rationale for this concurrency model? It seems complex for what we're trying to achieve at this stage."
If I were on the receiving end of that feedback, I'd much rather have the reviewer write something like:
"Can you walk me through the rationale for this concurrency model? It seems complex for what we're trying to achieve at this stage."
First, experienced people often develop mistaken beliefs. They try something and it happens to work, even though it might even be wrong in principle. Without any rationale, how is the person you're talking to going to judge whether the beliefs you've developed are correct?
Second, even if they're willing to trust you, just getting the rule handed down does not grow them as an engineer. I might prefer to make a mistake and learn than take the right path without understanding the reasons. In fact, I have done so, intentionally.
So, again, IMO, the goal you've mentioned in this comment ("figure out how to tell them `trust me...`") is an incorrect goal, and one you should not pursue.
You could ask questions that lead them to the same conclusion (with the benefit of hindsight from your experience): "What happens if this system is terminated before the final write call completes?" "How does this handle someone accidentally starting another instance of the application at the same time?" "If this external service we use is down, what happens?"
You could also walk through the story of your prior mistake, draw parallels to the new code, and then discuss how to avoid it.
"Why use XYZ pattern instead of ABC? This seems more complicated than necessary right now."
There's also a useful communication/pedagogy technique I learned many years ago called "praise, correct, praise". You frame a "correction" or negative feedback with either neutral or positive feedback. The idea is that the other person receives the feedback without feeling "stung" or associating it with negativity, which could reduce their receptiveness or even their ability to retain and/or apply the information.
At the end of the day I feel like our goals should be aligned to create the ideal balance of software for the larger goal. They should also be able to think critically about the problem I see and the solution I might offer.
If the person seems to be doing something boneheaded over and over after I’ve spent literally days, weeks, or months explaining easier and better solutions, and I keep getting forced to deal with it, then I may well conclude there are no extenuating factors and “this code is shit” just because of the person’s ego.
If somebody starts asking me to ‘walk them through the code’, that’d be a red flag to me. If this is someone technical, then I’d expect them to be able to follow the code themselves. If this is someone non-technical, then I expect that giving them knowledge is going to be more dangerous than useful.
It’s also disingenuous and feels like it creates a sort of prisoner’s dilemma. If I’m open about the pros and cons and details of my ideas, but they’re closed and use persuasive language to advocate for their own ideas, I don’t feel like it’s really a collaborative process anymore. They’re taking a route that gives them a distinct advantage, not because their ideas are better at meeting the overall goal, but because they’re tapping into people’s subjective preferences and drives to further their own self-interests.
Lots of companies do have this culture, but the end result seems to be to create an echo chamber where a blind eye is turned to vast inefficiency. Oftentimes this seems to be things that are hard to quantify like user experience and reliability or security, where you silently lose customers without a ticket being opened, so there’s no business pushback to bad technical decisions that hurt the product.
"Can you walk me through it" is also something I would ask to a student, not to someone whom I'm sharing the workload with.
If someone says it's too complex, it probably really is.
If someone says it's too complex, probably it also mean that maintaing that code could prove hard in the future.
So, unless you prove there are real, tangible, measurable benefits, it's better to find a different, simpler, implementation, no matter the rationale.
Consider a PR using mutexes instead of channels in a go project. Is the code in the hot path, and has it been benchmarked? How many mutexes are there, and are they hidden from external callers? Is the PR implementing an existing algorithm, or an algorithm with some desired behaviour which would be more difficult with channels? Which way is consistent with the rest of the codebase?
This is not an adversarial encounter between a defender of the codebase and a filthy peasant seeking to corrupt all that is pure and good, it's two programmers trying to solve a problem, and neither is likely to have 100% of the available information, so you should talk to each other and work out what's going on. Assuming that you're right will bias you towards your existing opinion, which will cause worse code to make it to production.