What is meta? ×
Meta Stack Exchange is where users like you discuss bugs, features, and support issues that affect the software powering all 130 Stack Exchange communities.

With a growing number of more or less inexperienced programmers on Stack Overflow, it is frequent to see the questions containing code not only badly formatted, but also badly written. While editing a question to fix the formatting is okay, I'm wondering if it is correct, when editing a question, to bring small changes to the code to make it more readable. In other words, until when is it acceptable to do a modification of a piece of code in a question?

Example

Let's say there is a question with following crappy C# source code:

void DoStuff( string filename ) {
          String MsgToDisplay = "";
           Stream stFile = new FileStream(filename);
          if (stFile.Length > 0)
          {
               Boolean result=this.validateFile( stFile );
               if (result==true)
                   MsgToDisplay="your file was submitted";
               else
                  MsgToDisplay="the file is invalid!";
           }
           return MsgToDisplay;
       }

State 1: The first thing to do is a general cleanup just to make it readable:

void DoStuff(string filename)
{
    String MsgToDisplay = "";
    Stream stFile = new FileStream(filename);
    if (stFile.Length > 0)
    {
        Boolean result = this.validateFile(stFile);
        if (result == true)
        {
            MsgToDisplay = "your file was submitted";
        }
        else
        {
            MsgToDisplay = "the file is invalid!";
        }
    }

    return MsgToDisplay;
}

State 2: The next thing may be to make some changes according to the style guidelines for C# and other minor changes which does not affect the code: proper case, bool instead of Boolean, etc. The original author can still recognize her own code and if she understands well the code she's written, she will understand the new one.

private string DoStuff(string fileName)
{
    string messageToDisplay = string.Empty;
    Stream stream = new FileStream(fileName);
    if (stream.Length > 0)
    {
        bool result = this.ValidateFile(stFile);
        if (result)
        {
            messageToDisplay = "Your file was submitted.";
        }
        else
        {
            messageToDisplay = "The file is invalid.";
        }
    }

    return messageToDisplay;
}

State 3: The last thing to do: make the more substantial changes by bringing things the original author may not know if she's a beginner:

private string DoStuff(string fileName)
{
    string messageToDisplay = string.Empty;
    bool? isFileValid = null;
    using (Stream stream = new FileStream(fileName))
    {
        if (stream.Length > 0)
        {
            isFileValid = this.ValidateFile(stFile);
        }
    }

    if (isFileValid.HasValue)
    {
        messageToDisplay = isFileValid.Value ? "Your file was submitted." : "The file is invalid.";
    }

    return messageToDisplay;
}

My thoughts

Until now, I thought it may be a good idea to go until the state 2, but never do the state 3. If we do the first two states:

  • It leverages the quality of the content of Stack Overflow in general and shows that this is not a sort of Q&A website where you can post questions without bothering to format them correctly
  • When other beginners are reading well-formatted code, they will probably try to write well-formatted code outside Stack Overflow too
  • The original author will see that her code was reformatted and maybe make an effort later to format the code by her own. She can also wonder herself why String was transformed into string, why there is a if (result) instead of if (result == true), etc

I had a lot of pain to go to the state 3, because:

  • It changes code so much that the original author may not understand why her own code was replaced by some stuff she doesn't even understand.
  • The modified code may be quoted in the answers. When the original author will try to link the answers to her own, original piece of code, she may not understand an answer, and will blame the answerer, instead of me (or the person who modified the question).
  • There is a risk of changing the original algorithm, just by mistake.

On the other hand,

  • I see a lot of answers which start by enumerating the incorrect parts of the source code: forgotten usings on IDisposable objects, possible SQL Injection, etc. I don't think those things have their place in the answers.
  • Again, keeping questions containing code with SQL Injection and without appropriate usings reduce the quality of Stack Overflow content. It is especially important since many beginners copy-paste code from Stack Overflow questions/answers without understanding it, nor reading the whole page with all the answers and comments.
share|improve this question
17  
Step one only. Maybe step two, on a case-by-case basis. There is nothing wrong with answers addressing code styles. That's how the asker learns from their mistakes, as well as future Googlers. It's also the only place where you can explain why the alternative style is useful or preferable. –  Cody Gray Apr 24 '11 at 0:29
5  
I was thinking of editing the code in section 1 of this question - it looks badly indented. –  amelvin Apr 24 '11 at 0:30
    
+1 for asking this question, even though I think you go to far (and +1 for @Cody) –  Donal Fellows Apr 24 '11 at 9:11
6  
Never introduce a horizontal scrollbar! ;-) –  Arjan Apr 24 '11 at 9:39
2  
Can I ask why you use "she" instead of "he"? It's not the first time I see this done but it confuses the hell out of me every time, and I have to stop reading for a bit. Why not just use "he" like normal people do? –  Andreas Bonini Apr 24 '11 at 12:35
    
@Kop: see the related question and my answer: stackoverflow.com/questions/3207257/… –  MainMa Apr 27 '11 at 13:15
4  
But that brace style is ugly as sin! –  APerson Mar 7 '13 at 14:44
    
"some stuff she doesn't even understand" -- do you have a specific asker in mind? –  Jan Dvorak Jul 13 '13 at 17:11
    
@JanDvorak: nope. I believe this applies to many beginners. Imagine changes like moving the JavaScript code to module pattern or playing with boolean logic to make a long statement more readable or use ternary operators or even dispose objects in C#. For all those techniques, beginners may be quickly lost, because they haven't learnt them yet. –  MainMa Jul 13 '13 at 17:43
    
I prefer OTB. –  Johannes Kuhn Aug 8 '13 at 21:12
2  
@AndreasBonini: I am intrigued - why do you consider one entirely arbitrary pronoun superior to another entirely arbitrary pronoun? –  unixsmurf Sep 1 '13 at 22:09
    
@unixsmurf: because it's the one that is used by convention. To make a programming analogy, using "she" is akin to writing function names uppercase in a language (such as C#) where they are conventionally written in camel case. It's not necessarily wrong, but it definitely is strange and annoying to the reader. –  Andreas Bonini Sep 2 '13 at 13:13

7 Answers 7

up vote 37 down vote accepted

My gut feeling, specifically for questions not answers, is that unless it's just a case of highlighting code as code, so that it displays nicely, you shouldn't change the code.

I'm open to persuasion on this, but the big drawbacks I see are:

  1. Repeat 'offenders' won't learn to change their posting style, as someone community minded is doing it for them.
  2. It won't encourage the poster to improve their coding style for legibility. This is something most of us (unless we're tidy by nature) learn this the hard way, and IMHO that is a valuable experience.
  3. Code compression using short-conditonals (test?true:false), implicit tests (if(blah)) and short-circuits (fnA()||fnB()), whilst great for getting more code on screen can hide the logic from the less experienced.

It all boils down to classic 'teach a man to fish' - tell posters where their code is wrong, but don't fix it for them.

share|improve this answer
10  
"1 . Repeat 'offenders' won't learn to change their posting style, as someone community minded is doing it for them." true, but not doing that will also lead to the same result, as he won't know how bad it is to post style like that –  ajax333221 May 21 '12 at 23:05
    
I'm afraid that quite a lot of posters ask one time only and then they leave, sometimes without even giving feedback on the answers. –  Cristian Ciupitu Jul 10 at 18:24
    
Changing the code in the question prevents the responses from filling up with nitpicks and coding suggestions, which detract from the actual discussion. Fixing the question demonstrates to the OP how to write a question, and without polluting the responses section. –  Moby Disk Oct 14 at 15:47

I think that reformatting the code so that it indents nicely and so on should be the end of the edit.

Improving the grammar and spelling of the actual question is one thing, but changing code is a minefield.

There is the possibility that an editor could add an unexpected bug into the code listing - which could be very confusing to the OP if answers suddenly start picking up on something that is completely new. But even without adding a bug, you could be inadertantly answering the question by your change (could changing a primitive fix a bug?, not sure) - which again would be confusing when people can't reproduce a fault.

share|improve this answer

Changing the format of the code, when it makes the code more readable, is welcome and useful.
Changing the code is different, for two reasons:

  • You could change what doesn't make the code work, and who reads the questions would not understand why the user is asking the question, or what is wrong with the code
  • The OP would have some difficulties to understand how to map the code proposed in the answers to the code he finds in his files

If by modifying the code you mean to write the correct name of a function, a class, or a method, that is fine if:

  • It is not what causes the code to not work
  • The user already written the name of that function, class or method correctly in the same code snippet

If by modifying the code you mean to change the code as you did in your "state 3" example, then I would say it is not fine at all. I would rather write an answer to the question, and suggest the OP how to better write the code he wrote.

share|improve this answer

I'm a rampant reformatter. There, I said it. I'll happily do reformatting that changes nothing but whitespace, with a focus towards making it clearer when read, with less scrolling side-to-side. I rarely go any further than that though; it's not my question or answer, after all. Substantial refactorings belong in answers, not edits.

But if I discover in the process that the problem in the question is actually down to the formatting (rare, but happens) then I will not reformat, and will instead point out in an answer (or occasionally a comment) what the problem is and how things should be fixed.

share|improve this answer
    
Since the implementation of the “anyone can propose an edit” feature, I've been editing less: usually someone else will propose what's needed before I get to it. Instead, I'm doing a lot of reviewing of edits. Aaaaargh! Meta-work will break me yet… –  Donal Fellows Aug 9 '11 at 23:36

I wouldn't do any more than a minimal clean up of indentation and the occasional line breaks. Substantial change around choice about brace style and purely stylistic (I would say your first edit is a not a full improvement: but I like the one true style).

Other than that leave the question alone. But any refactoring as part of your answer. That way before and after can both be seen.

(On SO I often vote to reject proposed edits that include anything that changes the question or answer. Better to answer again with a better answer.)

share|improve this answer

I think it is OK to reformat the code so it become easier to read. But it is not a good idea to actually refactor it. Or at least not within the question itself.
That said, you can always answer the question presenting refactored code and explaining why the original one is crappy. That would bring some value and at the same time will teach Original Poster something (assuming that they will even attempt to read your answer).

I believe that is exactly what StackOverflow is all about...

share|improve this answer

Good question!

Some (I presume a lot) could answer in an single edit really, which would of course make the question/answer redundant, the whole idea as far as I can see, whether home work helping, or rep whoring... is to never go past "state 1" with an edit, because to do so in an edit is presumptuous - you might already know the answer, but it's like every other quiz show on the planet... It's only easy if you already know the answer.

You cannot teach people the whole lot in one go without assuming an awful lot. I keep seeing the high reputation points users still banking on that in quiet times... But it is helping? or is it just that "need to keep my rep up" thing?

Another point is that if she were really following some questions where edits were out with the "why did you do that" reach, it then becomes a copy/paste solution... Is that always wise?

share|improve this answer

You must log in to answer this question.

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