11
\$\begingroup\$

I've noticed that, for whatever reason, seemingly every new user of Clojure tries to use def to create local variables. I've noticed it here, and on Stack Overflow, so it seems very pervasive. It's a fairly serious error, (def creates unscoped globals), so it's something that pretty much must be covered.

This has led to many of my reviews opening with basically the same warning, and the same examples of why it's bad, and what should be used instead.

How should this be handled? Should I be writing the same thing over and over in every review? Should I really go to town on one review describing the behavior of def, then link future reviews to that?

\$\endgroup\$
1

4 Answers 4

8
\$\begingroup\$

"How should this be handled? Should I be writing the same thing over and over in every review?"

You could, but I doubt it would be a productive use of your time.

You could also, however, make the same basic remark and link to one of your own explanations on a similar question instead. As long as that answer is on a well-received question, similar enough and, quite a must, on the Stack Exhange network1. High-quality answers would then explain the problem based on the actual code provided while supported by the (lengthy) explanation in the link. So your last remark:

Should I really go to town on one review describing the behavior of def, then link future reviews to that?

Is quite acceptable.

Feel free to use a format to base your answers on. Quite some regulars have experimented with such a thing in the past.

1 Not doing so could turn your answer into a 'your answer is in another castle' answer.

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

I keep a text file of my most used excerpts

enter image description here

that I can pull from for common mistakes. e.g.

Integers - [integers are obsolete](http://stackoverflow.com/questions/26409117/why-use-integer-instead-of-long/26409520#26409520). According to [msdn](https://msdn.microsoft.com/en-us/library/office/aa164506(v=office.10).aspx) VBA *silently* converts all integers to `long`. 

[Standard VBA naming conventions](https://msdn.microsoft.com/en-us/library/1s46s4ew(v=vs.140).aspx) have `camelCase` for local variables and `PascalCase` for other variables and names.

I tell them what is wrong, a brief reason why it is wrong and links to read if need be. Another example is

Comments - ["code tell you how, comments tell you why"](http://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/). The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe *why* you're doing something rather than *how* you're doing it. Here are a [few reasons](http://programmers.stackexchange.com/a/254979/125931) to avoid comments all together.



I could easily write a blog post on my website that explains the why or link to another answer of mine - but then they need to sift through the rest of the other answer to find what I'm talking about.

I could link to someone else's why e.g.

Mathieu Guindon (Microsoft Excel MVP, 2018) [reasons](https://rubberduckvba.wordpress.com/2018/01/12/progress-indicator/) that blah blah

For beginner VBA questions I always feel like it's important to give a reference as to what I'm talking about because the majority of beginner VBA questions usually don't have any background in coding in the first place (myself included some 5 years ago).

For something like c# I might want to link to a Jon Skeet answer.

And maybe for something as basic as "you can use such and such namespace" I'm sure I could find a beginner tutorial or even a reference to an article or book that can help with the why of the basics - not the how.


I want to give the asker as much information as they could want even if they don't want it, for the same reason I want to explain what I can at other sites - for future visitors.

It's not like saying

Look at [link]

but more

you should do this because of reason. If you want more reference see [link].

I'm still giving them the why and if the link goes dead my statement still works.


Just to be clear

My answer would look something like


For Each x in Collection 'iterate through items

You didn't type your variable x. When you don't define your variable, VBA will declare it as a Variant, which are objects.

Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.

You also have a comment telling me exactly what the code is doing, which should be evident. Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

If it's not clear, maybe you should improve your naming - give your variables meaningful names.


The only parts that I didn't copy from my document are in italics

That's four snippets from my text file which would stand alone without the links, but the links would be very helpful for someone that wants to know why. I'm also using their code as the example.

Obviously this only works for common beginner mistakes, but that's what we're talking about here. This is just works for me in this particular language.

\$\endgroup\$
6
\$\begingroup\$

Here is a concrete example of how I review a similar problem in C++.

Twelve ways that I respond to using namespace std; (twelve as of my writing this, I may write more that will show up in that search in the future).

Basically, I say something like

using namespace std;

You may have already seen Why is “using namespace std;” considered bad practice?

Sometimes I add more information that is specific to that review. Sometimes I don't.

I don't know if this is the best way to handle it, but it makes what I consider a usable compromise between explaining a complicated subject at length and just stating it. The long explanation is behind the link. The short statement of "Don't do this" is included in the review. And sometimes it seems worth additional explanation in the review itself.

You may note that I'm linking to Stack Overflow instead of Code Review. This is because SO gives canonical answers and Code Review reviews code. No one is posting just

using namespace std;

as a Code Review question. If they did, we'd close it as lacking context. However, that's a perfectly valid code block for an SO question. They don't want context in their questions.

\$\endgroup\$
4
\$\begingroup\$
  1. We don't really allow canonical question and answers. If you want to highlight something you can't whip up some example code and post a Q&A.

    Bit annoying, but it will prevent a wave of crap on the site.

    We were going to share common improvements on GitHub. But we've not got much there.

  2. Explaining everything is long. And I don't think everyone wants to know why.

    And so I've stopped explaining why as much any more. I've had maybe one user ask me to provide why, and so I add the why to the answer. Leading to a somewhat 'lazy' answer.

    I may be writing some poorer answer due to not explaining why. And so if you merge this with (1) you could just include a link to the information why, and not suffer any reduction in quality in your answers.


I now write answers more like:

  1. Follow PEP8.
  2. Don't have bare excepts.
  3. Use with when using open.

with open('file', 'w') as f:
    try:
        f.write(1 / int(input()))
    except ZeroDivisionError:
        print("Zero can't be the denominator of a fraction.")
\$\endgroup\$
4
  • 2
    \$\begingroup\$ "And I don't think many people want to know why." Make it optional, put it behind a link. Makes the answer short while still providing an explanation to those inclined. \$\endgroup\$
    – Mast Mod
    May 15, 2018 at 16:50
  • \$\begingroup\$ @Mast I said that at the end of 2 \$\endgroup\$
    – Peilonrayz Mod
    May 15, 2018 at 16:51
  • 2
    \$\begingroup\$ If you replace "many people" with "everyone", we're in agreement. I think quite a lot of people come here to learn and want to understand why they should do something suggested in the answers. Or perhaps I'm expecting too much of our visitors. Or humanity in general. Anyway, first (in 2) you point out many people don't want to know, then you explain you stopped telling people unless they specifically ask for it and then you appear to say you could include a link up front. Which approach are you recommending? \$\endgroup\$
    – Mast Mod
    May 15, 2018 at 16:57
  • \$\begingroup\$ @Mast I've changed the wording. It should be clearer that I'm saying what I do, and why. I also have said it has a potential drawback, and how you can 'fix' that. \$\endgroup\$
    – Peilonrayz Mod
    May 15, 2018 at 17:09

You must log in to answer this question.

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