# Code Review Clippy

A while back, y'all asked for some warnings for folks asking questions here: What would Clippy say?

After thorough review (mostly by animuson & JNat), we've determined that the following warnings can be implemented, based on question title matches:

\b((do(ing)?\s+this)|(this\s+(\w+\s)? code)|me|my|I)\b


This matches over 1% of the last 10,000 closed/deleted titles. All the rest of the expressions struggle to match this combined - I mention this because even if these warnings help in the cases where they show up, they're not gonna show up that often.

^(How|Can|Should|Would|Need)\b.+\??\$


Titles that contain a specific question are generally inappropriate for Code Review.

\b(wrong|incorrect)\b


Code that produces incorrect results is off-topic for Code Review.

\b(better|improve(ment)?|improving)\b


Please ensure that the title states what your code does. Seeking improvement on all aspects of the code is implied for Code Review questions, so specific concerns need not be mentioned in the title.

\b(short(en|er|ening)?|optimi(se|ze|sation|zation)|efficien(t|ce))\b


Efficiency and optimization are implied for every question on Code Review. The title should simply state the task accomplished by the code.

\b(refactor(ing)?|avoid|too many|need|DRY)\b


Please ensure that the title states what your code does, and reviewers will point out what needs to be improved.

\b(statements|loops|else|catch)\b


Please ensure that the title states what your code does. If the code is a purely generic example that has no specific purpose, then the question would be off-topic for Code Review.

I've gone ahead and turned these on. Enjoy!

• Thanks! I'm going to test this a bit now. It'd be interested to see the stats after this has been in effect for a while. – Jamal May 20 '17 at 4:36
• @Jamal here's the 90-day "before" picture =) (...it's ok to share this here, right?) – Mathieu Guindon May 20 '17 at 4:57
• This is really great, thank you! It seems the two top-voted suggestions in the linked meta post are not addressed. Can you please comment on that and the roadmap? – janos May 20 '17 at 5:10
• Curious about the 1% - is that against the last revision, or revision 1 (OP)? \b(wrong|incorrect)\b could probably include fail|failing too. – Mathieu Guindon May 20 '17 at 14:53
• It looks like "best practice" is still allowed, as with this question. – Jamal May 21 '17 at 2:19
• Would it be possible to get \b(wrong|incorrect|invalid)\b applied to all body text, not just the title? Obviously it shouldn't prevent submission, but it should at least address the seemingly widespread ignorance. – Cody Gray May 21 '17 at 11:12
• I tested against revision #1, @Jamal (then tested against the latest revision of not-deleted questions to see what the false-positive rate would be, and forgot to report it. Roughly 1% across all patterns, FWIW.) – Shog9 May 23 '17 at 22:57
• probably a LOT of false positives there, @Cody – Shog9 May 23 '17 at 22:59
• Top two are currently possible with minor changes (right now there's no way to limit body checks to questions, so a code-check would flag a LOT of answers) and possible with, uh, a tremendous amount of work and maybe not even then (past attempts to create pattern recognition based quality-checks on Stack Overflow have not had a great success rate, and the training set is considerably smaller here) @Janos – Shog9 May 23 '17 at 23:01
• Can you add "bug" and "not working" codereview.meta.stackexchange.com/q/8586/31562 to the Clippy for \b(wrong|incorrect)\b? – Simon Forsberg Jan 18 '18 at 18:05

Yay! Thanks!

Just one note, if I would see this as an entirely new user:

Titles that contain a specific question are generally inappropriate for Code Review.

I would be very confused. If I would come here from other SE sites, this goes against how most other SE sites work. I think it would be good to specify what we do want here. Something like:

Titles that contain a specific question are generally inappropriate for Code Review as we all want to improve our code in some way. Please tell us what your code does in the title.

• @Shog9 Yes. That's good. – Simon Forsberg May 24 '17 at 11:38

## That's awesome!!

Thanks a million! This will certainly help improve question quality on the site in a major way.

The only tiny little quibble I have is that it seems the forecolor for the x button and message text were somehow inverted, which makes the message a bit hard to read, and ironically puts subliminal emphasis on, well, the "dismiss" button:

### Actual

As rendered in my Chrome 58.0.3029.110 (64-bit) desktop browser:

• It also appears to cover the 'How to ask', which, ironically, is what we need new users to read the most. Especially the ones almost posting off-topic questions. – Mast May 20 '17 at 9:21
• @Mast all the better: first thing they see when they dismiss the notification is that it was covering an how to ask panel! – Mathieu Guindon May 20 '17 at 13:35
• Fair point. Perhaps it works as the big red arrow towards the help we've been hoping for. – Mast May 21 '17 at 7:02
• This color-inversion thing is happening globally on Stack Exchange sites. I noticed it the other day on Stack Overflow. It affects all pop-up boxes, and is probably unrelated to this change. Haven't checked to see if it is reported on a global Meta. – Cody Gray May 21 '17 at 11:10
• Yeah, the color thing has been reported a few times; donno what's up with that. – Shog9 May 22 '17 at 23:37
• Thanks to Hynes, this should be fixed after the next build. – Shog9 Jun 15 '17 at 21:40

Would it be possible to also add the ability to detect code-only posts. I don't know how common they are, but this question is a fairly good piece of code to review, but doesn't contain any text about what it's supposed to do, the motivation for writing it, or anything at all about it. There's already a close vote on it. I hate to see a good question get closed when a simple reminder could have prevented it.

• Yup, this one is possible - there's a ratio check that attempts to mandate a small amount of not-code for every line of code. That's enabled as of right now. – Shog9 May 23 '17 at 23:08
• Awesome! Thanks! – user1118321 May 24 '17 at 3:43
• @Shog9 Is there also a mechanism for requiring code to be in the question as well? – Der Kommissar May 30 '17 at 18:01
• Not without also requiring code to be in answers, @EBrown (which would hit a lot of answers here right now). Could be done, but would take a bit of work; got it on a list. A very long list. – Shog9 May 30 '17 at 20:55
• @Shog9 Can you perhaps make that a fixed number instead of a ratio? – Simon Forsberg Jun 26 '17 at 14:12
• No, although there's a fixed minimum number of characters for the post as a whole, @Simon. What's the need here? – Shog9 Jun 26 '17 at 14:12
• @Shog9 It was just a thought I got from this comment. I don't know the exact amount of code that the user tried to post, or the amount of non-code. – Simon Forsberg Jun 26 '17 at 14:53
• @Shog9 We just got another one from this comment, can we perhaps reduce the ratio a bit so that not so much non-code is needed? – Simon Forsberg Jul 1 '17 at 10:01
• @Shog9 I think the amount of text required to understand some code doesn't grow linearly with the amount of code. It grows slower. It seems that questions with longer code have a hard time adding enough text to meet this new requirement. If this rule cannot be adapted better to longer code, I'm afraid it would be better to disable it. – janos Jul 22 '17 at 21:04
• @Shog9 actually, as 200_success pointed out, to catch code-only posts, any non-zero ratio could be useful. So we could make the ratio small instead of disabling it. – janos Jul 23 '17 at 14:27
• Pretty sure I can't go below 1 char per line, @janos. Currently it's at 6 (the default) - if you want to change this, link a feature request. – Shog9 Jul 23 '17 at 18:05

I will just suggest a small improvement on one regular expression.

Currently, you have this:

\b(wrong|incorrect)\b


To display the following message:

Code that produces incorrect results is off-topic for Code Review.

But, I propose the following (indented for readability):

\b(
(?:(?:help|can(?:'t|\s*not)
|pl(?:ease|z))\s+)fix
|(?:not\s+|in)correct
|wrong
|throws?\s+exceptions?
|broken
|breaks?
|(:?does|can?)(?:n't|\s*not)\s+(?:work|output|display|present|run|compile|open|show|match)
|(?:(?:has|can|causes?)\s+(?:(?:performace|size|memory|display)\s+)?)(?:issue|problem|erro)s?
)\b


This should match things like:

My code doesn't compile
Can't display picture
Can't open file
This code doesn't work on IE6
Help fix exception being thrown
This code causes problems displaying PI
Broken display when running program on 800x600

This may also generate some false positives. Possibly far more than the over-simplified version you present.

This matches, possibly, some common titles when asking questions about broken code, but also matches some that may show up in the future (possibly rarely).

• Looks mostly good to me, but I'm a little concerned about the exceptions one because there are legitimate questions about working code and exceptions, like "Multithreaded foobar that throws exceptions on <some condition>". They're not asking about an error, but describing what the code does. Other than that, I like it! – user1118321 Jul 6 '17 at 1:33
• @user1118321 Thank you for your comment. I've been thinking about it and, in my opinion, changing throws?\s+exceptions? for throwing\s+exceptions? could fix it. What do you think? – Ismael Miguel Jul 6 '17 at 12:59
• Possibly. It just doesn't seem like talking about exceptions is enough to trigger such a warning in my mind. I can think of titles like "rewritten foobar code without throwing exceptions" or something along those lines. But maybe a scan of existing titles of closed questions would tell us for sure? – user1118321 Jul 6 '17 at 15:29
• That's a really good idea, but I have no idea of how to do it :/ – Ismael Miguel Jul 6 '17 at 20:16
• Unfortunately, I don't either. Sorry! – user1118321 Jul 7 '17 at 1:07