16
\$\begingroup\$

This suggested edit was rejected. As it should be - rolfl here has the correct reason:

Whitespace "fixes" should be part of an answer, not an edit. Please do not fix code problems in the question.

Example:

public boolean newCombo(int c, int l){
    if(grid[c][l].sample().isBasic()){
        addPieceToCombo(c, l);
        return true;
    }
    return false;
}

Was fixed to

    public boolean newCombo(int c, int l) {
        if (grid[c][l].sample().isBasic()) {
            addPieceToCombo(c, l);
            return true;
        }
        return false;
    }

As it was in a class.

Editing the indentation here is what I wanted, so that's great. Adding whitespace between if and (grid[c]... is not what I wanted.

Also, for this specific question, if you take a look at the code on GitHub... you'll find the indentation in working order. So in this case, it's the result of bad question formatting, and telling the asker to indent their code is not going to help them one bit.

If you want to get rid of the growing pile of zombies you'll have to find a way to deal with messes like this - we can't just say "That's a code edit, you can't edit that", because when I see a massive pile of badly indented code, then I can't review the code! (Alternative solution: I post an answer on these kind of questions: "Your indentation is bad, you can fix it with auto-formatting in your IDE, shortcut for eclipse is this, shortcut for netbeans is that, if you have bad indentation people can't read your code.")

Is it possible that we can fix the indentation in these kind of questions and maybe get some quality reviewing in, or should we just keep saying "bad indentation, use auto-format"?

\$\endgroup\$
  • \$\begingroup\$ I am fairly certain that there exists meta questions about this topic already. like this one --> meta.codereview.stackexchange.com/a/1816/18427 \$\endgroup\$ – Malachi Nov 4 '15 at 18:33
  • \$\begingroup\$ @Malachi and my post is not about that specific sub-topic, see the first few lines of my question, where I say that yes, you can't just "fix" spaces or syntax errors. But what about bad copy pasta? When the original is just fine? \$\endgroup\$ – Pimgd Nov 5 '15 at 8:32
26
\$\begingroup\$

There are times when it is obvious that people (new folk?) don't know how the markdown works on Stack Exchange. They take their block of code and paste it in to the edit box (and indent nothing):


import java.util.Scanner; public class MyClass {

public static void main(String[] args) {
    ///do something here
}

}


Some of those people fix their indentation by adding 4 spaces just to the first few lines, and you get:

import java.util.Scanner;
public class MyClass {

public static void main(String[] args) {
    ///do something here
}

}

Note the trailing } on its own line outside the code block. Also note that the main method and class declaration are at the left margin. This is a common "half-fix".


Other people see that trailing } and indent it as well, to become:

import java.util.Scanner;
public class MyClass {

public static void main(String[] args) {
    ///do something here
}
}

This is the state that the code in the OP's question was like.


The correct fix is to indent everything:

import java.util.Scanner;
public class MyClass {

    public static void main(String[] args) {
        ///do something here
    }
}

Ctrl-K makes it easy.... but, that's not an obvious thing in Stack Exchange.

When I see problems like the above example, I identify it as a "markdown problem" not an "indentation problem".

I have no problem fixing markdown problems (in the code, or not). I encourage others to fix them as well.

If the fix is to the indentation and other whitespace, and not just the markdown, then it is a code edit.

I realize the difference is subtle, but, it is OK to fix those times when people don't know how the SE Markdown works, but it's not OK to fix those times when people indent their code badly.

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

I have fixed the indentation for this specific question for the following reasons:

  • Without proper indentation, the code is hard to review
  • OP is new, and has only posted 1 question on StackOverflow before this one, which also had bad indentation and required editing by other users.
  • OP had issues with our rules, first having to edit to include the relevant parts (instead of 1 link to github, more github links) and the later pasting the code in (in one edit)
  • The GitHub link, when followed, will show properly indented code, even when looking at the repository as it was before the question was posted.

This makes me think that

  1. It's a benefit for the review process and the asker if the code was indented.
  2. The asker intended to have the code indented.

As for general policy, feel free to answer with policy proposals on that. I'm just stating the actions taken & justifications thereof for this specific question.

\$\endgroup\$
  • 3
    \$\begingroup\$ Well, 1 does not count, because that's potentially part of the review. 2 is an indication that OP might not have learned anything about the site, so might merit closer scrutiny. 3 is the same as 2, the plot thickens. 4 now is conclusive, and fixing the post so the code is indented identically to his sources is the right thing, though should be justified in the edit-comment especially if peer-reviewed. \$\endgroup\$ – Deduplicator Nov 3 '15 at 1:03
-1
\$\begingroup\$

If it's going from no indentation:

public boolean newCombo(int c, int l){
if(grid[c][l].sample().isBasic()){
addPieceToCombo(c, l);
return true;
}
return false;
}

To any acceptable indentation syntax I don't mind the edit, since any consistent indentation is better than none.

But within each language there are usually more than one school about how to do the indentations and when to use brackets and how to use them etc. that all boils down to what the individual dev prefers to read/write. If we're gonna go around and edit people's indentations we're gonna have a war over opinions.

My personal feeling is that code-review shouldn't go about editing the questions' code, we can comment about it and review it, but I feel like the edits should primarily be to improve the questions around the code to clarify what the asker means.

\$\endgroup\$
  • 5
    \$\begingroup\$ in my mind it really depends on whether or not the zero indentation is because of a copy-paste error or whether or not OP genuinely didn't use indentation (or used indentation inconsistently). \$\endgroup\$ – Dan Pantry Nov 2 '15 at 10:24
  • \$\begingroup\$ Inconsistent indentation can be quite easily noticed, however one should ask if the lack of indentation is intentional or not. If not it's something that really needs a review. \$\endgroup\$ – Gemtastic Nov 2 '15 at 10:35
  • 2
    \$\begingroup\$ @DanPantry And how can we tell the difference between copy-paste errors and real use of lack of indentation? \$\endgroup\$ – Simon Forsberg Nov 2 '15 at 10:54
  • \$\begingroup\$ @SimonForsberg copy-paste errors generally tend to be pretty obvious and the user has consistent indentation and then a part of the code is indented completely differently - usually either the start or tail end of the code. I've done this myself a few times where I paste in code into the question editor and then only highlight a portion of the code by mistake when I click the "Code" button. \$\endgroup\$ – Dan Pantry Nov 2 '15 at 11:00
-3
\$\begingroup\$

Only very carefully

When it can be put down to copy/paste errors in the same way as a typo. For example, a Python question with wrong whitespace but which from the question is obvious works as intended.

In non-whitespace relevant languages, I'd say never, since it's a proper part of the actual code review in those cases, and it's better to assume they did it wrong when you have no evidence to the contrary. (and in the python case you do have that evidence, as the code worked as intended.)

\$\endgroup\$

You must log in to answer this question.

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