How far can I refactor the code in someone else's question? [closed]

Question

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.

Answer 1

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.

Answer 2

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.

Answer 3

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.

Answer 4

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.

Answer 5

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.)

Answer 6

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...

Answer 7

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?

Answer 8

My view is that code readability matters, especially because it frees mental energy to focus on the purpose of the code rather than on its representation.

From my own experience on Stack Overflow I add that one attempt to apply PEP8 style guidelines to a Python code has been rejected on grounds of blatant superfluity. My starting point is that Python Enhancement Proposals are a product of a committed and competent community, thought for a general, mutual benefit.

My successive attempt to raise a discussion about this rejection has led to several downvotes garnished with allegations of fundamental freedoms being restrained, a few red herrings, and attempts to evoke feelings of guilt for flirting with fluff and for speaking from a self-referential bubble.

So the attitudes on Stack Overflow even towards 'state-1' changes, in the wording of Arseni, can vary considerably. See Conventional improvements of readability are not deemed to make the code "even a little bit easier to read" for the context. With the evidence I have, the topic is still fairly controversial.