'Canonical' questions to help address common issues

Question

Short Version

I believe it would be valuable to create a 'canonical' question/answer of sorts for each language, and I am hoping to get others' thoughts on this.

These questions and answers would serve as a central explanation of certain common issues. All answers would still be expected to be tailored to the posted code, but the canonical answers would provide a detailed explanation of sorts. These answers could complement the specific review to reduce repeated explanations of common (beginner) issues.

---

Long Version

In the two languages in which I am active on here (PHP, C++), there is a distinct set of common, core issues. I would go as far as to say that most questions are about 85% overlap when you consider the root issues, not the bits of code that contain the issues.

For example, go find 10 random PHP questions. I can guarantee you that at least 7 of those 10 will have an issue with globals. Additionally, each answer will have a different explanation ranging from a well focused, catered look at how it negatively affects the reviewed code to "globals are bad because I said so." Though these brief, seemingly opinion-based statements like this tend to be at most moderately helpful, I find myself doing it all of the time. It's simply too taxing on me as an answerer to continually explain the same things over and over again with just different variable and function names.

If a language-specific, central explanation of issues existed, it would mean a large decrease in the amount of "boilerplate" review necessary on the more beginner level code snippets. It would mean instead of a three page explanation of the harm of globals, I could send direct the person to an example answer and point out how it applies to them specifically. (e.g. "[globals are bad]. For example, $db is a very common variable name. What if some other piece of code overwrote your $db? Or, what if you wanted to be able to use the same function against 2 databases?)

---

What does this mean?

If the community agrees with me, it would mean people from each language getting together, carefully crafting a realistic question of purposefully non-ideal code and then creating answers to point out each of the issues.

The question would ideally cover as many common style/bad practice/inefficiency/etc issues as possible without being sacrificing a minimal level of readability and decipherability.

In turn, each answer would focus on one very specific issue and explain, in detail, what the issue is, why it is an issue, and how to fix/avoid it.

---

What does this not mean?

This does not mean that I would support answers simply becoming "look at the following N links."

Countless books on code quality exist. Our entire community's knowledge could be summed up by "go learn SOLID really, really well, then go find the champion for code quality of your language, and read everything they've ever written." Our value--I believe--is in our customization. We don't just tell generic tips in an abstract, contrived form, we point out very specific, very real issues in a question asker's specific code. This would of course continue to be the case. The canonical answers would function as the "why" not the what. We would still need to relate the items to the code posted, not just point them out blindly.

---

Other thoughts

In a way, this is an exercise of DRY and single source of truth, and the benefits are similar. Duplication would be avoided, and best of all, as languages and best practices change, a central explanation would allow us to better reflect that. A great example of this is C++'s use of std::size_t. Until fairly recently, it was considered best practice to match the standard library's use of std::size_t when dealing with containers or working with sizes of your own. Recently the suggestion has became "use signed numbers unless you truly need that extra bit" due to the more natural semantics of signed numbers. There are quite a few C++ answers around on CR (including a lot of mine) that suggest using std::size_t. At some point I will go back and fix all of these, but until I get around to it, it would nice if they linked to a central location that noted this change of best practice. Although the parts of the criticism unique to the question would stay dubious, at least there would be an explanation at the other end of the more general link explaining that things have changed and why. It would keep the misinformation at a minimum until time permits.

Answer 1

Combining a few things here....

Tag wikis.

Absolutely. The tag wiki should be used as an entry point to 'faq' type issues for that language/tag.

Using meta....

Absolutely not.

Single Question for all issues with one tag.

Like this.... for C#

No. All questions should conform to the on-topic requirements. This one does not.

Solution?

Seems easy to me:

• Have one question, tagged appropriately, that shows a single common CodeReview 'issue' (for example, C++ and using the using namespace std; common theme)
  • question should be community-wiki
• have people review the question 'like normal'.
• accept the 'right' answer
• update the tag wiki index like.

This pattern is how it is done on SO. Consider the Java tag wiki, where there is a FAQ section near the bottom.... and follow the links, to posts like this one: Avoiding NULL statements in Java

---

Example wiki section for C++

---

Common Review issues:

• using namespace std; when you should use your own namespace
• other 'faq' items.

Answer 2

For those unaware, there's an old Meta question that did the same thing for Python:

Python - Common Improvements

It received a historical lock specifically because it didn't belong on Meta in the first place (doesn't pertain to the site itself), but it was also too valuable to delete. It's also too old to migrate, so if it should be added to this collection, it would have to be recreated on the main site in some way.

That's the main reason why I've recommended against keeping these entries on Meta. Another reason is that syntax-highlighting won't work on Meta, as it was demonstrated in the above question when it was originally tagged as python to hopefully acquire the proper highlighting.

Answer 3

As you've seen, I've experimented a bit. Sorry for jumping the gun and going ahead like this, I hope not too much damage was done. I've given this lots of thought lately, and I come to a conclusion that's radically disagreeing with the idea.

Should we have such CW questions/answers?

No. It's useless, redundant and counter-productive.

Ok that was brutal. It's not completely useless, there's real value in these posts. The intention of this post is to address leaky abstractions, but it turns out it's also about the Unit of Work design pattern, and there's probably some valuable wiki-grade information that could fit in there; the OP's code could be fleshed up a bit, and the title changed and another reference answer could be posted.

Here's the counter plea.

---

Useless.

If I'm going to be posting some code on Code Review, I'm probably not going to browse the site's review-wiki questions (probably even less so tag wikis) and change my code accordingly with what I read in there, before I post my first question on the site.

---

Redundant.

After a while being active on the site, I end up with a number of pretty good answers under my belt, and I'm noticing that a lot of programmers encounter similar problems, and I'm starting to find that I'm repeating myself.

If I find myself re-writing [part of] an answer that I've already given (or that I have seen elsewhere on the site), then I'm doing it wrong.

What I should do instead, is link to that existing answer (whether it's from me or from another reviewer) and make myself a legitimate and encouraged way to get more eyes on my previous work... and collect the random upvotes and badges later.

Every once in a while, I should go back to my answers history, re-read my posts, and edit and add links as needed.

Which leads us to...

---

Counter-productive.

We're doing this for the money imaginary Internet points.

Yes, we all love that warm-fuzzy¹ feeling we get when we get someone their "ah-HA!" moment. But a beta site doesn't drive on love and fuzzy feelings. We need to keep growing, and to do that, we need to generate reputation points. That's just how it works!

By making these posts Community Wiki, we're ultimately only slowing ourselves down, it's not productive. I mean, not just because of the race thing.

Let's just link to our existing excellent answers. Linking to an answer gives it more attention, and can get you a shiny new badge; there's possibly also a badge for the author of the question and another for the author of the linked answer.

We have real users with real questions demanding real reviews, the time we'll spend coming up with all the fake questions and answers to build a solid CW "knowledge base", is time we will not be spending on the real stuff that counts. And every single vote spent on a CW post, is a vote that will not generate any reputation score on the site, that takes the place of a vote that could.

_{¹#warm-fuzzy hashtag, StackExchange-style.}

---

Answer 4

This thought occurred to me as well, after writing this exception-centric answer, this SOLID tutorial and this comment-centric answer.

I completely agree with @svick's comment:

This sounds like a good idea to me. If we go forward with this, such questions should probably be marked as community wiki.

After reviewing code for a while, it turns out a lot of common code issues and smells are repeated in various different ways, in many answers throughout the site.

If I take the "don't throw System.Exception" answer by itself, I can easily, and probably will, reuse it in the future - by linking to it. Same with the comment-centric answer.

Don't throw System.Exception (see this answer for more information).

Of course this alone wouldn't make a very valuable answer/review, but it can easily be a point in a series of observations about someone's code.

---

The question would be an exemplary CR question, with working code, a good title and enough context for readers to fully grasp everything that's going on. Ideally the code should be small enough to not be a wall-of-code question (i.e. no scrolling code blocks), but should contain all the "common flaws" to be covered in the answers. Something that covers all of this (and more? what am I forgetting?):

• Single Responsibility Principle (class or method does way too many things)
• Exception/error handling ("swallowing" exceptions, throwing System.Exception, etc.)
• Naming & conventions (PascalCase vs camelCase, cryptic/bad/misleading names, etc.)
• Bad scoping (declared but not used, declared way too far from usage, globals, etc.)
• Comments (misleading, redundant, wants-to-be-funny, etc.)
• Arrow code (nested conditionals, ideally in a nested loop?)
• Bad indentation

These posts would be CW anyway, so we could always edit and add more crap-code along with a CW answer that "reviews" it.

---

I think this is a great idea, that can greatly enhance the overall usefulness and searchability of the site. Let's do this!

(doesn't it feel like "extracting a method review"?)

Answer 5

Tag wikis are repositories of language- or topic-specific information. I think they could be perfect for checklists like these. As an example, the sql tag wiki currently has a checklist of items to include in a good question.

Tag wikis are currently an underused feature of the site, and may be a bit obscure. However, that's a bit of a chicken-and-egg problem: the more we build the tag wikis, the more awareness of them builds. In addition, we could suggest a change so that the [tag:python] syntax links to https://codereview.stackexchange.com/tags/python/info instead of https://codereview.stackexchange.com/questions/tagged/python.

Furthermore, within each checklist, we can link to specific topics such as srp.

Answer 6

Instead of one artificial question with several carefully-crafted answers, you could:

• Wait until a real question is posted with one of these "common, core issues"
• Post a carefully-crafted answer which addresses one issue
• Add a link to your answer, to the corresponding language-specific tag wiki, as suggested in 200_success's and rolfl's answer.

---

Example of answer titles/topics for one language (C#):

• Use using to Dispose disposable local variables <-- hyperlink
• Make your class IDisposable to Dispose disposable instance data <-- hyperlink
• ... etc ...

Answer 7

Contrary to what Mat's Mug wrote, I think the canonical question should not contain any code. I think that each answer should be independent and that when someone clicks on the “see here for more” link in a normal answer, they shouldn't be required to read lots of code that is not relevant to that issue.

Also, if I want to add a new canonical answer, that approach would mean I have to figure out some way to introduce the issue I want to write about to the code in the question. If there's going to be 20 answers (which seems like a plausible number to me), I think the code in the question would have to be quite convoluted to show all 20 issues.

So, the question should contain just a short intro explaining what it is and no code. If example bad code would be useful in some answer, it can be introduced right there.

The fact that such question wouldn't normally be considered on-topic doesn't bother me. We, the community, decide the rules for what's on-topic, so we can also decide if there are going to be exceptions to those rules.