12
\$\begingroup\$

Is reviewing generated code on topic ?

Scenarios:

  1. Someone wrote the generator and it is included or linked and provided with generated code.
  2. Someone wrote the generator and only included generated code for review.
  3. Someone used a third party generator and posted generated code for review.
\$\endgroup\$
  • \$\begingroup\$ related: meta.codereview.stackexchange.com/questions/5011/… \$\endgroup\$ – RubberDuck Mar 11 '15 at 17:38
  • \$\begingroup\$ Also related, this answer indicates it may be off topic. \$\endgroup\$ – RubberDuck Mar 11 '15 at 17:40
  • 4
    \$\begingroup\$ Why don't you just ask the question and we'll see what we think about it and make a decision based on your specific question rather than trying to come up with a "one size fits all" general policy? \$\endgroup\$ – Simon Forsberg Mar 11 '15 at 22:39
  • \$\begingroup\$ @SimonAndréForsberg yeah that's a good idea. When I do post it I'll add a link to here \$\endgroup\$ – bhathiya-perera Mar 13 '15 at 3:47
  • \$\begingroup\$ Hey, why did I get a downvote ? \$\endgroup\$ – bhathiya-perera Mar 18 '15 at 14:49
  • 1
    \$\begingroup\$ @JaDogg I downvoted because there is not much context about what the generator is, what kind of code is generated, etc. There are way too many "What if" scenarios here. \$\endgroup\$ – Simon Forsberg Mar 24 '15 at 16:13
12
\$\begingroup\$

If we were asking about any generated output other than code, the answer would be an obvious and resounding "No, you can't judge a program based on its output."

And I know that's not exactly what's being asked here, but that's essentially the intent behind the first two scenarios described in this question. In these cases, we're stuck with a user who has put the cart before the wagon. He's written an app to generate output... but he's unsure what the output should look like.

So, realistically, what should be reviewed, is a sample of what the generator might produce.

If you're writing code-generation code, you need a very concise, specific goal as for what the generated code should look like. And I'm completely fine with seeing a project created for the sake of hammering out what the output of the generated code should look like. And in this case, that it is generated code is irrelevant (and should be left out of the question).

From here, once the template for the generated code has been reviewed and hammered down, and the developer creating the code generation software is satisfied with the output, then the code generation software should be based on its code--not on the code it outputs.

In no other circumstance would we consider it acceptable to review a piece of code based on its output--why should output that happens to itself be code be any different?

\$\endgroup\$
12
\$\begingroup\$

No, it's not. It's not the generated code that's reviewable, but the input that was used for generating it.

I have a lexer and a parser generated by ANTLR4 in C# ..but I didn't write that code, and even if I put it up for review, and even if it does get reviewed, then what? I'm not going to modify generated code and rename all the generated methods to PascalCase because that's how C# code should look like - these changes will be lost as soon as I regenerate the code!

However the grammar file, if I had written it myself (or the parts I wrote myself), would be suitable for a code review; reviewing that grammar file requires knowledge of the generator tool (here ANTLR), so no one reviewing it is going to suggest naming my grammar rules in PascalCase (to generate C#-ish method names) because they know the tool has a Java background and will not even recognize the grammar rules if I do that.

Knowing what the generator input (like that grammar file) is supposed to be doing helps reviewers - if I say "that grammar takes VBA code for input", then reviewers can assume that as working code, the grammar effectively produces a working lexer and parser that can process VBA; asking for ways to fix issues would be off-topic - I couldn't ask how I could change my grammar to recognize commented-out code, for example.

But I could ask about the naming of the grammar rules (which is crucial here); I could ask about redundant or convoluted ways to achieve what the grammar is doing and whether/how that could be improved.

But the generated code? Reviewing it would only waste everyone's time.

\$\endgroup\$
  • 1
    \$\begingroup\$ TL;DR: Review the code that's doing the generating. \$\endgroup\$ – RubberDuck Mar 12 '15 at 20:19
  • 1
    \$\begingroup\$ TLDR? It's a phone post! \$\endgroup\$ – Mathieu Guindon Mar 12 '15 at 21:27
  • 2
    \$\begingroup\$ lol.Just leaving a condensed version... \$\endgroup\$ – RubberDuck Mar 13 '15 at 0:21
7
\$\begingroup\$

It depends...

What generated code are we talking about?

Are we talking about a whole game that has been generated? Good luck in making such a generator in the first place. But even if you did, I would have to say: No. I don't consider this really "your code", even if you did write the method or not. You didn't write the code.

Are we talking about a generated constructor? I use my IDE all the time to generate constructors, toString methods, equals and hashCode implementations, etc. Are such code on-topic? Absolutely. (At least I don't see any close votes on my questions...)

A question with parts of the code generated? Sure.

A question with all of the code generated? No thanks.

\$\endgroup\$
  • 1
    \$\begingroup\$ What if I write a T4 template or a CodeDOM construction and want to know if the generated code conforms standards/best practices? Why would this suddenly be offtopic? \$\endgroup\$ – Jeroen Vannevel Mar 11 '15 at 21:21
  • \$\begingroup\$ @JeroenVannevel I honestly don't know what a T4 template or CodeDOM construction is. \$\endgroup\$ – Simon Forsberg Mar 12 '15 at 14:39
  • \$\begingroup\$ Take this code for example: pastebin.com/YNwkymgK. This is a CodeDOM tree that will generate a .Equals() implementation based on some parameters that I pass in. Why would I not be allowed to show you the resulting output and ask whether the implementation follows conventions? \$\endgroup\$ – Jeroen Vannevel Mar 12 '15 at 14:45
  • 1
    \$\begingroup\$ @JeroenVannevel What kind of options are available for the whopping two arguments passed into that? Essentially, you're asking us to review the code you generated using this to determine whether or not you're passing the right arguments... \$\endgroup\$ – nhgrif Mar 12 '15 at 16:46
4
\$\begingroup\$
  1. Definitely yes. The generated code can be reviewed as part of the generator's review to help improve the generator.
  2. If the question is otherwise on topic, I don't see why not. We can review the code for best practices, and the person can adjust their generator and its output to conform to the reviews' suggestions.
  3. No. Unless the generator's code is open source, they can not modify the generator to conform to the suggestions, anyway. Also, it is off topic for the same reasons other third-party code is.

In other words, I think we can review the generated code as long as the feedback is directly going back to improving the generator (possible with first-party generators).

\$\endgroup\$
  • \$\begingroup\$ Why review a program by looking at its output rather than reviewing the program itself? \$\endgroup\$ – nhgrif Mar 11 '15 at 21:30
  • \$\begingroup\$ Because reviews on the program would probably focus on the program itself, not its potential output. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Mar 11 '15 at 21:34
  • \$\begingroup\$ The output is part of the program. \$\endgroup\$ – nhgrif Mar 11 '15 at 21:34
  • \$\begingroup\$ What if it calculates its outputs based on some input? The output isn't immediately visible then. As far as I know, this is well into the future, but a real possibility. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Mar 11 '15 at 21:37
  • \$\begingroup\$ How is that different from basically any other useful program? \$\endgroup\$ – nhgrif Mar 11 '15 at 21:38
  • \$\begingroup\$ My first response was that the output was a real program that might need to be debugged and maintained. However, once the generator is fully written, will the code need to be maintained, or will it just get told to output a new program when a new release is planned? Probably the later, so I guess the output doesn't really need to be reviewed. I still think the first option is on topic, though. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Mar 11 '15 at 21:44
  • \$\begingroup\$ If the generator is generating code that needs maintenance, is it working as intended? \$\endgroup\$ – nhgrif Mar 11 '15 at 21:45
  • \$\begingroup\$ If the output needs debugging, it isn't working as intended. However, I can see the output needing maintenance - especially if only one person/company owns the debugger and charges a large fee to output code. Perhaps they will generate a modern OS that needs to be customized/maintained by the buyer for cheaper than it would take to develop from scratch, but too expensive to not maintain and continue developing from scratch. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Mar 11 '15 at 21:50
4
\$\begingroup\$

I personally feel that if we are having to look at output to 'review' code, then we are not really 'reviewing' code, but rather determining that the code that generated the output works correctly. And in that respect, that indicates (to me) that the generator code is broken and thus it is off topic for code review.

\$\endgroup\$
3
\$\begingroup\$

Generated code is not code that you have written, especially if the Generator is a 3rd-Party application/generator.

I would say, no.


If you wrote the code for the Generator then the output would be to show that the code(generator) that you wrote works. only in that instance would a question including Generated code be on topic.


The other question, is code Generated by a 3rd party generator my code?

  • can I take ownership of Generated code?
\$\endgroup\$
  • \$\begingroup\$ But generated code is a result from something you create. That's why it's generated in the first place. Assuming this, it will always be ontopic because I don't see a scenario where the generated code is independent from any configuration you do prior to generating. \$\endgroup\$ – Jeroen Vannevel Mar 11 '15 at 18:02
  • \$\begingroup\$ It looks like you stopped in the middle of a sentence? \$\endgroup\$ – svick Mar 11 '15 at 18:14
  • \$\begingroup\$ I removed that, I kind of see where Jeroen is going with his answer \$\endgroup\$ – Malachi Mar 11 '15 at 18:33
0
\$\begingroup\$

Yes, it's always ontopic.

Either you write the generator yourself or you configure the generator in a way that it creates output the way you want it.

In both scenarios you have a direct influence on the resulting code and as such warrants a review. This counts for the generator code, the configuration code and the generated code.

\$\endgroup\$
  • \$\begingroup\$ in order to review the input to get the output we still need the inner workings so we know what is best for input to get the output we desire. right? \$\endgroup\$ – Malachi Mar 11 '15 at 18:54
  • \$\begingroup\$ Sort of. It's the same as any other library: you use the API available. There is no inherent need to know the inner workings of the generator itself. \$\endgroup\$ – Jeroen Vannevel Mar 11 '15 at 18:57
  • 2
    \$\begingroup\$ I'm going to start wall-of-coding CR questions with the XML that my interface builder creates. \$\endgroup\$ – nhgrif Mar 11 '15 at 20:51
  • 3
    \$\begingroup\$ And I don't mind at all, just don't expect a review from it. There are many different kinds of code generation and I'd rather not place any restrictions instead of white- or blacklisting specific ones. \$\endgroup\$ – Jeroen Vannevel Mar 11 '15 at 21:00
  • 1
    \$\begingroup\$ Your last point in the comments is a good one. I'm just uncomfortable reviewing output. The output should be highly specified, right? Wouldn't reviewing generated code really either be a spec review or some kind of strange design review? Idk. I need to chew on this one. \$\endgroup\$ – RubberDuck Mar 12 '15 at 0:58

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .