@ChrisStratton (1) The proposed change can be partially evaluated. You can review the code quality by itself, but you cannot evaluate the quality of the solution it provides. (2) Breaking the build is not an argument here. I can write perfectly valid code that replaces all "e"s with "y"s which will compile, but that doesn't mean that it's an appropriate solution to an actual problem. Or, when a bug is encountered with a too long string, I could write a fix that truncates the string instead of changing the allowed string length. That does not mean it's a good fix to the problem.

@ChrisStratton Your comment effectively argues that "if it builds, it's good code", which is an obviously flawed premise.

@ChrisStratton: You've just agreed that the code can be flawed, and therefore Rick cannot blindly trust Morty's proposed fix. If Morty doesn't explain the actual problem, Rick doesn't know what the actual problem is. As this memory leak could be an edge case, there's no reasonable expectation for Rick to be able to reproduce this with no information abot the problem whatsoever. Morty's "help" is actually making Rick's life more difficult. If he had explained the issue instead of provide a fix with no justification; both Rick and Morty's lives would have been easier.

@ChrisStratton To prove the point, let's say Morty changed Console.WriteLine("Lgo file created") to Console.WriteLine("Log file created"). Rick's argument doesn't quite apply here, because (a) the original problem is easily inferable from looking at the original code (b) the solution is equally clear about what it fixes. But most real life bugs are not this simple, and in the posted example, the problem is not described nor easily inferred.

@ChrisStratton You're not distinguishing between reviewing the code and reviewing the fix. The question isn't just if the change compiles or even makes sense; the question is also whether it's the best solution to the problem. Without knowing the problem, you can't evaluate whether the proposed code is the best solution or not. Morty email effectively causes Rick to guess what the original problem was. And guessing is never a good approach.

The point is that you can evaluate if this change is right or wrong independently of if it solves the problem. Either the old code is inherently wrong or the new code is inherently wrong. If someone outside the dev team is sending inherently flawed fixes, it's worth a one-sentence explanation of the flaw the first time, and raising an organizational process issue if it's a habit.

@ChrisStratton: That statement is abjectly false. You seem to be unaware that two syntactically correct pieces of code can have different effects, and therefore be a wrong/right application of the required business logic. You also claim that it's possible to confirm solution to unknown problems, which is so obviously and simply wrong that I can't formulate any further explanation to your misunderstanding. I am not continuing this discussion.

Please use the chat room for discussions.

Great post, but I feel there is one fallacy/false assumption you make - He's asked to change something (and effectively take responsibility for having changed something) without getting any choice.. Who says he doesn't have any choice? I've been a developer for a long time, and I understand the "fix" in the original email to be simply a suggestion, an attempt. Like, he took a shot at finding the issue/fix... and the dev can take it for what it is worth...

@GWR: Note that I said "he is asked to change something", not that "he is required to change something". I agree with what you're trying to convey but I was referring more to Morty's expectations of how Rick will respond to his mail. Morty offers a solution, and does not adequately explain the problem it seeks to solve. The only possible inference is that Morty expects his fix to be used. Otherwise, he would've explained the problem even if he tacks a suggestion on at the end. The mail strongly implies that Morty was not focusing on pointing out a problem, but a solution.

I don't really understand above comment chain. The issue is clear: valgrind reports memory leak. From the patch it is clear memory leak is caused by this particular allocation, because using a smart pointer fixes it. Rick is free to fix it any way he wants, the patch just illustrates the problem really. Using unique_ptr is likely to be a better fix, and if modern C++ is used then one should certainly not use a naked new like this, so clearly the patch is not perfect as a fix, or maybe not a fix at all. But it is perfect at illustrating the problem!

I don't think this is a XY problem per say because Morty described the problem: a memory leak, and his findings. He isn't describing a solution to Rick, and Rick assumed it was a proposed solution. Most people would probably know what the problem was if they written the code and saw the diff.

The police-officer analogy is flawed. A more apt analogy would be "You're a police officer. A fellow police officer comes up to you and say 'I need you to arrest that man'". Of course, that doesn't support the point you're trying to make, because professional colleagues (aught to) have a greater degree of trust and respect.

@Dan I said it's a potential XY problem. Without a problem description, Rick is unable to assess whether the proposed fix is appropriate. Morty could be completely right, but the email does not prove that because it omits the core of the issue: the problem description. Also, Morty is proposing a solution: "...and the issue can be fixed with the below diff." That is a literal claim of having provided the solution, not just pointing at some relevant code.

@starchild The OP describes Morty as the system owner, not a coworker. OP and Rick are coworkers, Morty isn't. If Morty were equal to Rick, there'd be no point to getting Rick to implement a solution that Morty claims to have already solved.

@hyde When reporting a problem, you should report the problem. Not just show a possible fix and have the developer guess as to what you're fixing. This is a matter of clear communication. I'm not saying that Morty's fix is bad, I'm saying that without explaining the problem, Rick can't evaluate whether the proposed fix is good.

@Flater I guess I'm unclear on the distinction. Anywhere I've worked as a software developer, "System Owner" is a role independent of level, and everyone working together on the product is a "coworker".

@Flater regardless, "a man comes up to you" is more analogous to end-user bug reports than to a colleague, regardless of level.

@Flater The problem is explained. If you are not a C++ developer, it may be not be apparent to you, but any competent C++ dev sees it immediately from Morty's mail, as it is presented here. Now it remains for Rick to evaluate if using a smart pointer, and which smart pointer type, is the correct fix.

@Flater I like your police metaphor, but I'm not sure it's completely on point. I agree that Rick will probably have to look at the X anyway, but to reject any possible Y is still pretty rickdiculous. With your metaphor, I feel like it's closer to Civilian: "I need you to arrest this man, Jean Valjean, for breaking his parole. He's the mayor now, and he runs that factory right over there." Police: "Thanks, but in the future please just provide the crime, not suggested information. I don't read suggested information. I'll wait a few days before reacting so I forget all that."

Everyone is focusing too much on the specific problem. Anyone who has spent much time working a tech help desk or assisting less experienced devs or coached other engineers in using dev tools has seen much worse XY problems crop up. That's what Flater is calling out here, not necessarily this specific instance, where the proposed solution does hint at the underlying problem (I found a memory leak, and valgrind says this specific pointer is leaky).

The problem might be the monitor, but it might be something completely different. I've had to dig to find out what the problem is: turning on the computer results in a blank screen.

@Flater that said, you might consider clarifying that you're defending Rick's position generally, rather than referring to the specific case.