13
\$\begingroup\$

Background

The number of zombies have been on the rise. Lately we have almost stopped their growth, however they are still great in numbers.

There has been built many great weapons to combat this, see Jack of All Trades, Master of None - Let's work together to offer ultimately better content, faster or Call of Duty II - We're still on a mission. The idea is somewhat controversial, and hence would like to start an discussion on the matter before doing something more drastic.


The new weapon: Documentation

Many users including myself spend time writing the same improvements to users over and over again, like violating the DRY principle. Giving references to the same documentation over and over again in itself also violates the DRY principle.Searching up links to documentation and coding-principles takes time, time that would (imho) be better spent on improving the posters code.

Similar to Frequently Posted Comments, such a post would serve as a reference list to common documentation and practices that you often suggest in your answers. My idea is to have one answer for each language, and one answer for general principles. Both of these should just be at maximum a few sentences "bombarded" with links for further reading. They would then be followed by code snippets and your own hand-tailored suggestions.


What it is and what it is not.

Similar attempts have been tried before, see Create a reference for newbie common mistakes or 'Canonical' questions to help address common issues. However this is different as it is not meant to be used by beginners, but by us.

The idea is NOT to create copy-and-paste answers to questions and every principle and bit used should be paired with examples from the actual question. Nor is it an attempt to create less handcrafted answers, but more. Ideally when one spends less time writing about the general principles to follow, one can spend more time focusing on the deeper problems with the code, or new ways to improve it.

@Phrancis showed an example he often encountered in SQL

Your table aliases are not helpful. Single-letter aliases give no
information at all about what they are referencing. It's fine for
aliases to be short, as long as they carry some form of useful
meaning. For instance, [start critiquing the specifics of OP code]

While a comment that would be placed in the general principles section might sound like this:

Your main method is responsible for too many things. The main method is  
meant to be an entry point into your program, rather than the program 
itself. As a general principle try to let each function serve a single 
purpose ([Single responsibility principle](https://en.wikipedia.org/wiki/Single_responsibility_principle)).
It would be better to extract some of the code into 
[routines/methods/functions/classes and continue with OP code and examples]

Ideally though they should be a tad shorter. Another language specific snippet I see again and again is the quit() function in Python. The function is very natural for a beginner to try to use, however as documentation shows, in most cases it can and should be avoided.

A bad example would be linking just to PEP8 as it would be too general and not really give the OP much of anything. Instead the ideal solution would be to leave a short sentence explaining what PEP8 is and then add more concrete examples where OP fails to follow these principles. Is the naming convention unclear, is the indentation wrong, is the spacing between statements inconsistent, and so forth.

Some attempts at such a thread have been tried earlier. Python - Common Improvements, however it was closed for being unclear. I hope I have made myself clear with what this questions and answers try to achieve.


Question

After a bit of discussion in chat this is a very controversial topic. Some say that this would decrease the of receiving a hand-crafted review. While I would think the opposite, that it would increase the handcrafted parts, as less time is spent looking for documentation.

  • Could and should such a weapon be added to our arsenal?
  • Would this diminish the feeling of our personalized answers?
  • Could it streamline the process of writing answers for beginner level questions?
\$\endgroup\$
  • 1
    \$\begingroup\$ I'm wondering if the SE-comment script could be adapted so that each and everyone can maintain a list of his favourite paragraphs/links and have them available quickly. I've opened github.com/Benjol/SE-AutoReviewComments/issues/122 accordingly. Then these could be shared or not but I fear that we'd have most answers sound/look the same if we were to do it. \$\endgroup\$ – Josay Jul 25 '16 at 20:25
12
\$\begingroup\$

For context, whilst I apparently registered on the site in 2011 I've only really paid any attention to it for the last 3 or so months and I've been answering for less time than that which probably puts me still firmly in the newbie camp.

I'm not a huge fan of the canned response suggestion (from either side of the process).

From the reader's perspective:

  • If I start reading exactly the same sentence that I've read before it's going to make me tune out. There's a good chance that I won't get to the good bit at the end of it. For the particular OP that posts the code, a lot of the information might be new. However, for the regulars I think it will very quickly start feeling samey and boring.
  • I think it's also going to make reviews feel a bit disjointed, with this canned response bit sounding like '@Phrancis', the next bit sounding like '@N3buchadnezzar' and then a bit sounding like the actual person attributed to the answer.
  • I know one of your goals is to try to provide more space/time for the reviewers to be able to concentrate on the interesting bits of questions that aren't covered by the paragraph starters, however I'm concerned that it might lead to users simply rolling out reviews that consisted simply of any of the suggested paragraphs that were vaguely relevant and then didn't really go further into the code, essentially lowering the level of contribution, rather than increasing it.

From the writer's perspective:

  • I don't find lists like that particularly approachable (i'd rather type the same paragraph I've typed a hundred times in a slightly different way than go and find a canned response for that particular item). In most cases, between window switching, searching and copying it's going to be faster for me to just type it anyway. For it to work for me, I'd need to create a template that had all of the things I might want to include in the review, copy and paste the whole thing and delete/expand as appropriate. This would make the reviews look more and more alike.
  • I have my own style. I wouldn't say it's an overly polished style, however it is mine and I don't especially want to sacrifice it by using somebody else's snippets at the start (which is arguably the most important point) of any review.

So what's the alternative?

I don't have a stand out alternative. I have thought about creating a checklist of things that I would want to check in a review. I think this might be useful, in the sense of not forgetting things however If I'm honest, I probably wouldn't use it. When I'm reviewing code, I like to be led by the code I'm reviewing, that's part of what makes it interesting. If all it's variables are 'x,y,z' then it screams naming issues. If it's using lots of globals, then they become the dominant issue and I might not be as bothered by a few bad variable names. I think this is part of what's compelling about the site each review comes from the perspective of the writer which is based on their experience and approach to code. If we start relying on lists I really think we'll lose that.

The documentation beta on stack overflow is interesting in a sense. I can see how it could actually be very useful in the context of providing review feedback. Having examples that you could link to as part of reviews in a known place as part of 'further reading' could be useful. One of my few contributions to the sql documentation is a common mistake that impacts performance. Having a strategy for collecting something similar for 'examples of improvable code style', (for example 'names matter' comparing a well named section of code with a single letter named section of code) might be useful. It would be nice if this could be hosted on CR, however I don't see that happening any time soon.

\$\endgroup\$
2
\$\begingroup\$

Past reviews are documentation

When I'm working on some code, especially if it's in a programming language that I don't use very often, I often look at existing questions and reviews to gain some insights as to how to write decent code in that language. If every newbie did that before posting, I think we'd have more interesting code to review rather than the same old mistakes repeated over and over.

StackOverflow questions are documentation

I often refer to StackOverflow questions and/or answers when doing my reviews. To illustrate, for example, the pros and cons of a particular language construct or technique, it's very often useful.

Other sites are documentation

For anyone who would do so, there are a great many documentation sites (including devdocs.io) that provide reference material for different programming languages. There are also numerous blogs, podcasts, videos, etc. available for anyone who would take the time to look for them.

How I do reviews

First, I think it's important to realize that some people posting might either be new to programming or new to the particular language. For that reason they may or may not have looked at the other sources of documentation, and may or may not understand how it might apply to their code. That's why I don't think another documentation source will help.

When I do a review, the first thing I do is make sure that I have a block of time available to do it. That's usually the most difficult thing! The second thing I try is to look over the code to see general code quality, formatting, etc. If it's in rough shape, it's likely that it will be a longer review. If it's pretty good, it might be a shorter review, but I find that it often still takes about the same amount of my time to do.

I usually try to compile the code with warnings turned all the way up, run it under something like valgrind to look for memory leaks, and test it with my own example data. I also often rewrite portions and benchmark the before/after times to make sure that my suggestions are actually improvements.

Next, I look through my own personal collection of reviews that I've already done and compose the review offline. If there's a point I've already made a thousand times, I typically just copy it into the current review, modifying text as needed to make it relevant to the current code.

Finally, I look it over one last time and paste it into an answer, checking for formatting or spelling errors. When I've fixed any remaining spelling or formatting errors, I hit the "post" button and usually upvote the question at the same time if I haven't already done so.

Automation

I proposed an automatic code reviewer for a code challenge back in May 2015. I still haven't implemented such a thing, but think it would be both doable and useful, not as a replacement for human reviewers, but as an adjunct something like the way that many IDE tools will autogenerate skeleton code given basic project parameters. A human is still needed to add the particular pieces that make it useful, but much of the mundane and repetitive work can be automated.

\$\endgroup\$
  • \$\begingroup\$ I fully agree with this answer. \$\endgroup\$ – Mast Jan 5 '17 at 9:30
2
\$\begingroup\$

I understand the desire for this and respect the intent.

...although I disagree.

The only real reason is one I have not seen posted above.

sure, you will be re-phrasing a lot of the same points in most questions,

but I pride myself in using the reading of the questioners code to understand how far along they are in their education of software development.

I usually don't point out spelling mistakes and things because I don't think they are germane.

Some other reviews do a line by line breakdown but I personally try to tell if the questions true underlying question is

  • am I on the right track..even at all?
  • How can I improve?
  • I know what I am doing, I dare you to critique me! (in a good way)
  • I have hit an education brick-wall and need to be pointed in a direction?

specifically in the case of that last one, you can tell is someones education is:

  • lacking in Creation pattern understanding,
  • unaware of performance pitfalls
  • just touching on OOP principles for the first time
  • learning clean code and testing their understanding
  • have just read an architecture book and are over-zealously modularizing but without a rudder as to the true benefits.

In short, historically... sure, the points will always be valid but I don't think the answers SHOULD be documentation.

They should be the leg up the specific questioner needs.

When you can tell they are experimenting with something new, Praise their successes and guide them in the right direction if they seem confused.

maybe suggest some reading material out of scope of the question but clearly a preface to what their next experiment/question WILL be.

If they are just sanity checking their knowledge then pointing out reference material can be condescending. Instead bullet point their project as if it is a corporate code review.

... IMO.

so yeah, canned answers (even with the best intentions) are a no go for me.

\$\endgroup\$

You must log in to answer this question.

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