How should we review code written around a template?

Question

It is fairly common for review requests to be for homework or interview practice questions.

For example there's a fairly steady stream of python tic-tac-toe games which I'm basically convinced fall into this category. These tic-tac-toe implementations consistently have certain slightly strange design decisions, such as using separate functions for the two players and using mutual recursion to swap between them. Without fail, the reviewer will say something like "Your play_X and play_O functions are basically identical. You should rewrite the code to have just one play function and take a parameter." To be clear I completely agree with, and have given, this advice.

However, Occam's razor tells me that there is some teacher or textbook or course or something providing the template for the Tic-tac-toe game, complete with weird mutually recursive design decision. Potentially, half of our suggestions are irrelevant because the asker has to follow the template (although probably helpful from an educational perspective in suggesting criticisms of it) Further, it seems that at least part of what we're reviewing is not the asker's own code, which has implications with the rules.

Is there anything that needs special considerations in handling (known) template based questions like this?

Answer 1

TL;DR: IMHO no special handling of template questions required. Review the code the same, point out weaknesses of the design, and include the questioner's rational in your answer.

---

I say the code should get reviewed the same way. It should not be important whether code follows a template, because the essence of the review should not differ. With your example, a review would either note that the "functions should get replaced by a single one" or that "the requirement to write two functions is off, better use a single one and call them in both of those functions". The overall review still contains the same message, although the target of the criticism varies slightly (code vs interface).

However, if the author expects improvements to follow the (strict) interface and requirements, they need to include them in their question. That is not specific to homework or interview questions; legacy code can have the weirdest interfaces and requirements.

Further, it seems that at least part of what we're reviewing is not the asker's own code, which has implications with the rules.

We use standard libraries in our code. printf is not implemented by most of our C questions, and neither is any third-party library whose interface decisions we have to follow. It depends by case of course.

Potentially, half of our suggestions are irrelevant because the asker has to follow the template

They might be irrelevant for that specific case, but as you said, it is "helpful from an educational perspective". In the end, a good code review does not only improve the current code, but also the programmer behind.

If we point out where the overall design is malformed, the interested developer can solve the problem again with a new design, free of the template's burden. If we do not acknowledge the bulkiness of an interface or requirements, we acknowledge them, and we should try to avoid that.