28
\$\begingroup\$

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.

\$\endgroup\$
  • 4
    \$\begingroup\$ Related: meta.codereview.stackexchange.com/questions/482/… and meta.codereview.stackexchange.com/questions/491/… . It seems like it has been discussed before (a long time ago). I like the idea and have had the same thought many times. \$\endgroup\$ – Simon Forsberg Mar 20 '14 at 8:24
  • 3
    \$\begingroup\$ "Our value--I believe--is in our customization." - you're not the only one believing that. \$\endgroup\$ – Simon Forsberg Mar 20 '14 at 11:18
  • 1
    \$\begingroup\$ This sounds like a good idea to me. If we go forward with this, such questions should probably be marked as community wiki. \$\endgroup\$ – svick Mar 20 '14 at 13:19
  • \$\begingroup\$ Should we create a "wiki" tag for these questions, so they can be found easily? \$\endgroup\$ – Mathieu Guindon Mar 20 '14 at 14:26
  • 2
    \$\begingroup\$ @Mat'sMug SO has a C++-faq tag for such topics. \$\endgroup\$ – ChrisW Mar 20 '14 at 15:25
17
\$\begingroup\$

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:

\$\endgroup\$
  • \$\begingroup\$ That's an interesting idea, but I have reservations about requiring code in those questions: When I want to write a canonical answer about some issue, I don't have any specific code in mind. So I would write the question to fit the answer, which feels backwards to me. And changes to the answer might require changing the question. Also, it wouldn't technically be on-topic, because the code would be just an example, not "real code". \$\endgroup\$ – svick Mar 21 '14 at 16:28
  • \$\begingroup\$ @svick How about taking existing questions that represent typical conditions, and linking them as 'do this, it's best' ? \$\endgroup\$ – rolfl Mar 21 '14 at 16:47
  • \$\begingroup\$ Are you saying that codereview.stackexchange.com/questions/44877/… is off-topic? If so what do you suggest instead? Are you agreeing with my proposal? \$\endgroup\$ – ChrisW Mar 22 '14 at 3:05
  • \$\begingroup\$ I am saying that question is not a question, and it is off topic. I have upvoted your answer/suggestion because it keeps questions as questions, but I woud prefer the tag-wiki as the place where the 'index' is kept (because meta should be questions and answers too). \$\endgroup\$ – rolfl Mar 22 '14 at 3:07
  • \$\begingroup\$ +1 Having it in the tag wiki makes it easy to find when you answer a question about that language. \$\endgroup\$ – ChrisW Mar 22 '14 at 3:37
7
\$\begingroup\$

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 to hopefully acquire the proper highlighting.

\$\endgroup\$
7
\$\begingroup\$

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 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 1 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.

1#warm-fuzzy hashtag, StackExchange-style.


\$\endgroup\$
  • \$\begingroup\$ The more I think about it, the more I think CW is a bad idea. At the end of the day, we all like and need (from a beta perspective) rep. I am a bit confused though: are you saying a central repository is a bad idea period, or that a central repo should just link to personal answers (like have the language wiki have a bunch of links to exemplary answers)? (Basically have a community curated list of links rather than a community curated set of content?) \$\endgroup\$ – Corbin Mar 26 '14 at 3:49
  • 1
    \$\begingroup\$ I suppose we could have links to frequently linked-to answers in a language's tag wiki... maintained by whoever finds a good answer to link to. \$\endgroup\$ – Mathieu Guindon Mar 26 '14 at 3:53
  • \$\begingroup\$ @Corbin You can try to create a 'canonical answer', which for example: a) can address a single topic/problem (not every problem in that piece of code b) name the topic/problem well c) explain the problem in detail and also try to explain it in general d) contain hyperlinks to canonical references e) therefore be an exemplary answer or 'the last word' on the subject, which other people will want to link to in future, and which therefore deserves to be added to the tag wiki. For example `## Use standard casing conventions for identifiers\n\n(Explains that the language uses Pascal case for ... \$\endgroup\$ – ChrisW Mar 26 '14 at 10:22
  • \$\begingroup\$ ... this and camelCase for that; mentions why conventions are important; suggest tools for doing static analysis of style; add a hyperlink to the document in which these conventions are defined.` \$\endgroup\$ – ChrisW Mar 26 '14 at 10:23
  • 1
    \$\begingroup\$ MSDN would be the "last word" on the subject, that people can link to now and in the future... \$\endgroup\$ – Mathieu Guindon Mar 26 '14 at 12:30
  • 1
    \$\begingroup\$ My problem with simply pointing back to a previous (normal) answer post is that it is not single-topic. That guy's review will have all kinds of stuff. The artificial exemplar question would have perhaps one line of code. \$\endgroup\$ – JDługosz May 1 '18 at 20:46
6
\$\begingroup\$

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 .

\$\endgroup\$
  • \$\begingroup\$ The advantage of answers is that we could link to specific ones directly. Could we do something like that with the tag wikis? \$\endgroup\$ – Corbin Mar 20 '14 at 23:21
  • \$\begingroup\$ @Corbin Why wouldn't you be able to create links from a tag wiki? It's just Markdown. \$\endgroup\$ – 200_success Mar 20 '14 at 23:35
  • \$\begingroup\$ Oh, now I think I misunderstood. Do you mean we should use the tag wikis to link to the canonical (or, I later realized 'reference' is a better word) questions/answers? I thought you meant we should put the actual reference material in the wikis. \$\endgroup\$ – Corbin Mar 20 '14 at 23:59
  • 3
    \$\begingroup\$ I was thinking of enumerating the Do's and Don'ts in the tag wikis themselves. However, I'm just one person with one idea. I really believe that tag wikis should be involved in the solution somehow, but the exact form remains to be determined. \$\endgroup\$ – 200_success Mar 21 '14 at 1:56
5
\$\begingroup\$

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"?)

\$\endgroup\$
  • \$\begingroup\$ Bad scoping might get difficult... there is a multitude of subcategories... \$\endgroup\$ – Vogel612 Mar 20 '14 at 14:38
  • 3
    \$\begingroup\$ -1 because I vote to use multiple real questions and write single-topic canonical real answers, instead of one or more fake questions. \$\endgroup\$ – ChrisW Mar 22 '14 at 16:54
5
\$\begingroup\$

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 ...
\$\endgroup\$
  • \$\begingroup\$ FWIW I think this is thw way to go, for the reasons underlined in my 2nd answer. \$\endgroup\$ – Mathieu Guindon Mar 26 '14 at 22:11
  • \$\begingroup\$ If such a "natural" question exists, great! Let’s tag them with [exemplar] so I can discover them. If one does not exist, only then would I consider crafting a synthetic question containing only that one issue. \$\endgroup\$ – JDługosz May 1 '18 at 20:47
4
\$\begingroup\$

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.

\$\endgroup\$

You must log in to answer this question.

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