-6

Recently I encountered and answered the question How to split a string into chunks based on specific length?. The question title may seem trivial, the cause isn't.

That is because the user copied their code from this answer that currently has a score of 304 (+326/-22) on a question that's got 306,000 views:

static IEnumerable<string> Split(string str, int chunkSize)
{
   return Enumerable.Range(0, str.Length / chunkSize)
       .Select(i => str.Substring(i * chunkSize, chunkSize));
}

Please note that additional code might be required to gracefully handle edge cases (null or empty input string, chunkSize == 0, input string length not divisible by chunkSize, etc.). The original question doesn't specify any requirements for these edge cases and in real life the requirements might vary so they are out of scope of this answer.

It's ... less optimal code because apart from the missing parameter checks it silently drops the last chunk if the input string length is not % chunkSize. This is what the asker of first question, which I answered, was surprised by.

This restriction is not mentioned in the question's title, nor in its body, but only mentioned in a comment by the OP. It is also named a "side effect" which "might" require additional code in the answer.

Sure, sure, the onus is on the copier of code to check (preferably with a unit test) that the copied code does what they think it does, but no one knows how many times this code has been copied into production and caused grief (if it's even detected), while the fix is so stupidly trivial (yet it took me a couple of attempts to get right).

Three people including me have tried fixing the answer. The fixes keep the code functioning exactly the way it does for the current use case, but stops silently dropping the last chunk and starts returning it instead.

Yet the poster insists on rolling back edits, and instead added prose explaining how they don't want to fix their answer, as well as repeatedly doing so in comments.

What do we do here?

  1. Keep broken code at the top, doing nothing? (The second answer at +174/-0 won't rise to the top soon.)
  2. Fix the answer and lock it?
  3. Add a banner to the answer that more clearly indicates its brokenness?
  4. Something else?

Why?

Sidenote: this is one instance that for me proves that voting for quality doesn't work. 300.000 people search "C# chunk string", copy the highly-upvoted answer, it seems to work, 1 in 1000 gives an upvote, and then later on have to encounter the bug (you can't convince me that all upvoters only needed the str.Length % chunkLength == 0 scenario) and then fix it in their code but don't go back to turn their upvote into a downvote. Or they do, but can't, because their vote is locked in.

12
  • 4
    The answer already contains an explanation stating the code drops "rest" characters. The OP also asked other users to just post a new answer if they think their code isn't good enough. What I would suggest: Leave the code alone, and improve the explanation that was provided to more clearly explain the edge-cases.
    – Cerbrus
    Aug 25 at 9:03
  • 1
    Doesn't the second rated answer handle exactly the case where the string length is not dividable?
    – BDL
    Aug 25 at 9:04
  • 1
    @BDL what does that have to do with the first answer, sitting at almost double the score and actively causing problems in at least one but guaranteed more situations?
    – CodeCaster
    Aug 25 at 9:09
  • 1
    @Cerbrus why would you suggest that? What happened to the collaborativeness of this platform?
    – CodeCaster
    Aug 25 at 9:09
  • 6
    As a rule of thumb, you don't edit code on SE. Especially not with changes as significant as that. The author of the Q/A has final say in what the contribution contains. If they don't like your edit, they have the right to revert that. I'm suggesting an alternative edit that both solves the issue, and leaved the author's code intact.
    – Cerbrus
    Aug 25 at 9:12
  • 2
    @Cerbrus I know of the "don't edit code" guideline. I also know through experience, both as a developer and as a community member, that this answer I'm talking about is problematic and I want to fix it to make the community as a whole better. I do believe most of the votes on said answer to be misguided, and I do find the OP annoying in their stubbornness. But that's on me.
    – CodeCaster
    Aug 25 at 9:17
  • 2
    This answer seems perfect for people to adapt to their own requirements, just as the answer itself states is needed. For example, (silently!) getting a (last) chunk of wrong size may well be more undesirable than discarding it or doing something else. Why would the "fixed" version be unambiguously be more correct? Aug 25 at 9:22
  • 1
    @MisterMiyagi a developer walks into a bakery: "One wholewheat bread please". The baker asks: "Sliced?", and the developer says "yes". The baker hands them 2/3rd of the loaf, sliced. But thanks, I get that, just having chunks of equal length may be an equally valid requirement.
    – CodeCaster
    Aug 25 at 9:23
  • 2
    @CodeCaster A developer walks into a bakery: "Slices of bread, please." The baker hands them two Slices plus a PartialSlice. Aug 25 at 9:26
  • 1
    @MisterMiyagi more bread is more better.
    – CodeCaster
    Aug 25 at 9:29
  • "Three people including me have tried fixing the answer." - You forgot to mention 5(!) suggested edits with the same purpose: "fixing" the code. All of them have been rejected.
    – Tsyvarev
    Aug 25 at 19:02
  • @SecurityHound you can't ping a deleted user, and I'm not going to edit in a constraint which will invalidate a whole lot of discussions and answers and make the code worse by having it apply to the subset of the problem. Why are you suggesting that? Do you not understand that I'm talking about the original question there?
    – CodeCaster
    Aug 26 at 5:31

1 Answer 1

9

Popular answer only handles subset of problem stated in title and question

Actually, the popular answer handles the whole problem and specifies constraints when the solution is applicable. That is, I find nothing wrong with such answer.

There are other answers which provide less-restrictive solutions. The fact that the referenced answer is the first one (even with "trending" sorting) could mean almost anything, including:

  1. Many visitors doesn't suffer from its constraints.
  2. Visitors are able to adapt the simple(!) solution for their less-restrictive constraints without needs to read other answers.
  3. Visitors miss constraints, because the answer post insufficiently emphasizes them.

Only the last point seems "problematic" for me. But it could be solved by editing the question for highlight the constraints more, not by the changing the solution.

For me it is never a good approach to change a solution in the answer post just because the answer post is mostly rated by the visitors, and someone assumes their solution to be "the best" one.


Concerning the other question where OP tries to apply the solution when its constraints are not fulfilled. From my understanding such question should be closed as a duplicate with a comment like

It seems you have taken a solution from <the answer>, but forget about checking its constraints. In your case these constraints are not fulfilled, because <your string's length is not divisibly by the chunks' length>. You could choose a solution from other answers to the same question, e.g. <that one>.

9
  • 2
    There are no constraints in the title nor in the question, and the answerer added the clause that their code "might" require additional code to handle "edge cases" only after multiple people bothered them about it. It's just a shitty experience all around, and I don't like this "no touching code" purism that's standing in the way of pragmatism. The spinoff question and the voting and the comments tell me that too many people are surprised by the intricacies and that something needs to be done.
    – CodeCaster
    Aug 25 at 18:17
  • And the answer is not invalidated by adding the additional bounds checks and rounding, it will only be made more versatile while being kept backwards compatible. I really don't see the harm in editing here.
    – CodeCaster
    Aug 25 at 18:18
  • @CodeCaster: "the answer is not invalidated by adding the additional bounds checks and rounding" - If you are about that edit, then from my opinion it greatly changes the solution: casting integer values to float, rounding, computing minimal - all that complicates the answer. So, there is a real trade-off between additional constraints and more code. Why do not post separate answers with solutions which cannot be ordered by "better"? E.g. there is the answer which similar to your approach: stackoverflow.com/a/51687086/3440745.
    – Tsyvarev
    Aug 25 at 18:37
  • 2
    "the answerer added the clause that their code "might" require additional code to handle "edge cases" only after multiple people bothered them about it." - I fully agree with you that in case of absence of constraints specification the answer is not so good. And I would fully support anyone who would add such constraints to the post: only with constraints specification the answer becomes correct. But modifying a solution to the form which "better-ness" is quite subjective doesn't look like a correct way.
    – Tsyvarev
    Aug 25 at 18:46
  • It's a Linq answer. People using Linq aren't bothered about an extra cast or method call or two. As for why other answers don't work: do I really have to explain that people don't look beyond a highly upvoted, accepted answer? Oh wait, I did, right in my question.
    – CodeCaster
    Aug 25 at 19:28
  • "do I really have to explain that people don't look beyond a highly upvoted, accepted answer?" - Voting is NOT about indicating a single answer which is better than anything else. Voting is about indicating several answers, which have a high score, as most prominent ones. A reader could choose among those several answers according to their (reader's and answers') specifics. And the reader should definitely NOT to give up if the most scored answer doesn't work for them. I don't understand why you confront to closing the your-answered question as a duplicate...
    – Tsyvarev
    Aug 25 at 19:40
  • 1
    This whole discussion is about making the site better for people who search, not about voting. People who search for "c# chunk string" will find this question and have code on their screen that will only work for a subset of the problem that they searched for and that the question title contains. I know about the "don't edit code" guideline, I'm here to discuss when some leniency could be applied to its application in order to make the site better as a whole. I don't understand why that's seen as so alienating.
    – CodeCaster
    Aug 26 at 5:35
  • @CodeCaster: Ok, It seems I get the "core" of your suggestion. Please, correct me if I wrong. You have found the "popular answer" which provides the solution with constraints and that answer explicitly lists those constraints. But you suspect (with objective reasons!) that many readers miss those constraints. And this is a problem. You suggest to modify a solution in the popular answer to the form which doesn't have constraints. From your opinion, that modified solution would deserve those upvotes, which have been placed for the old solution by readers who miss the constraints.
    – Tsyvarev
    Aug 26 at 9:33
  • 1
    I think that sums it right about up, yes!
    – CodeCaster
    Aug 26 at 10:32

You must log in to answer this question.

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