56

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.

closed as off-topic by ale, Glorfindel, Robert Longson, rene, M.A.R. Jul 2 '17 at 19:54

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "This question pertains only to a specific site in the Stack Exchange Network. Questions on Meta Stack Exchange should pertain to our network or software that drives it as a whole, within the guidelines defined in the help center. You should ask this question on the meta site where your concern originated." – ale, Glorfindel, Robert Longson, rene, M.A.R.
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 20
    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
  • 7
    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
    +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
  • 10
    Never introduce a horizontal scrollbar! ;-) – Arjan Apr 24 '11 at 9:39
  • 5
    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? – Thomas Bonini Apr 24 '11 at 12:35
  • @Kop: see the related question and my answer: stackoverflow.com/questions/3207257/… – Arseni Mourzenko Apr 27 '11 at 13:15
  • 5
    But that brace style is ugly as sin! – APerson Mar 7 '13 at 14:44
  • 1
    "some stuff she doesn't even understand" -- do you have a specific asker in mind? – John 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. – Arseni Mourzenko Jul 13 '13 at 17:43
  • I prefer OTB. – Johannes Kuhn Aug 8 '13 at 21:12
  • 4
    @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
  • 2
    @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. – Thomas Bonini Sep 2 '13 at 13:13
47

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.

  • 13
    "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 '14 at 18:24
  • 1
    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 '14 at 15:47
  • The learning rate of the OP is irrelevant or can only seen as a side-effect, imho. StackOverflow should be seen as a resource that provides good solutions to problems in the documents. The questions and answers are therefore common knowledge. In my opinion, it is important that the questions and answers are also formulated accurately, do not contain any spelling and grammatical errors and are also formatted in an easily readable format. Why shouldn't this also apply to code? Sure -- changes to the code content are just as critical here as in text and should, in my opinion, also be avoided. – colidyre Aug 1 '18 at 11:35
35

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.

  • For example, the OP refactored stFile to stream in step 2, but didn't change all occurrences of the name. =~) – HaveSpacesuit Jul 31 '17 at 13:45
11

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.

10

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.

  • 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
4

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

4

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

2

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?

1

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.

  • "allegations of fundamental freedoms being restrained". Really? – Cai Jul 2 '17 at 15:54
  • @Cai Not entirely really, but somewhat close to that... For example, that of imposing 'personal preferences' (so personal as a PEP guideline, in fact) on the others' free will, hence of undermining their freedom to code as they like. Which is always possible and licit, of course, but it should not give leeway to blaming the application of an established guideline as a deliberate act of damaging the post authenticity. Hope this makes sense. – XavierStuvw Jul 2 '17 at 19:31
  • Saying you shouldn't edit to change the code style of a post (and pointing out that others may disagree with your choice of style) is not even slightly close to what you're saying. Where exactly did anyone accuse you of deliberately trying to damage anything? – Cai Jul 2 '17 at 19:43
  • @Cai The nearest I can think of are sentences like you going around editing code to align with your personal preferences rather than other people's means that you're harming them by making it harder for them to read their own posts .... you're saying that it is only your personal preferences that matter, and nobody else is allowed to differ in their own personal preferences? What makes you so special? Why shouldn't anyone else's personal preferences be taken into consideration? Of course, I may fail in my own choice of wording. The post's gist is that style enhancements remain controversial – XavierStuvw Jul 2 '17 at 20:14
  • I wish to add that eventually that post's owner accepted my (i.e. PEP) editing suggestions; and when I pointed out this occurrence in the meta discussion, I got an extra bunch of downvotes. So the learning attitudes of that community vary. – XavierStuvw Jul 30 '18 at 10:47

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