2
\$\begingroup\$

The comments on my question here appear to have been removed.

Some of these comments reported bugs in the original code. Given that question code can't be updated, and new questions or selfie answers are expected to mention those that provided helpful information, this seems self defeating at best, and designed to drive users away at worst.

Can anyone provide rationale for their removal?

\$\endgroup\$

1 Answer 1

7
\$\begingroup\$

In general, comments are ephemeral.... they are not designed to have long-lasting content. Comments are to seek clarification on posts, and point out short term issues. Once the issues are addressed, the comments should be removed.

In the case of this specific question, things became a real mess.... in part, because the question started off "broken", and then got fixed, and then got answered, and then got updated.

On Code Review, answers look stupid if they talk about things that are not visible in the question. We (as a community) decided years ago that the integrity of answers carries more weight than the integrity of questions - questions are supposed to have issues (otherwise Code Reviews would be boring), and answers are supposed to address them. The net of the decision is that edits to questions that invalidate answers are rejected, or rolled back.

In order to allow a meaningful progression of code after a review though, the concept of follow-on questions is encouraged ... i.e. fix the code in your own code base, and if you want, you can submit the revised code for further review as a new question.

Back to your specific question - the timing of things is important - your code had bugs, people commented on these issues, people voted to close (justifiable), and you edited the code to address these comments (hence, a bunch of the early comments were redundant, and should be deleted).

Then, the question got answers.... at this point, changes to the code in the question causes answer invalidation - even if those changes "fix bugs" in the code.

Then, later comments are just people going back and forth about what should be happening with code changes in question edits....

Right now, the question has (what I believe to be) working code (so it is on topic), and it has answers. If you want the revised code to be reviewed, post a new question.

If there are further bugs in the code that have not been resolved, then people should post their comments as answers, not comments.

Here's a dump of the (deleted) comments - there is nothing of significant value in there that I can see:

Your question is accumulating close-votes, presumably because the behaviour of the method is not defined, and there is no indication of how it would be used. You might consider adding additional information so that its purpose and usage are clearer. – VisualMelon Jul 26 at 20:07 deleted by rolfl♦ 1 hour ago Undelete

@VisualMelon You don't think it is fair to assume people capable of answering are familiar with the existing String.IndexOfAny? I would also suggest understanding the naive implementation should be perfectly clear to someone that could answer usefully, but I appreciate the advice, and have edited the question. Of course, I disagree that it is off-topic, it is in fact exactly one of the supported topics. – NetMage Jul 26 at 21:00 deleted by rolfl♦ 1 hour ago Undelete

Well, I didn't know what it did until I read your code, but that's not what I mean: you'll get better and more useful answers by providing a clear specification and representative example code, ideally from a real project, to put it in context. – VisualMelon Jul 26 at 21:05 deleted by rolfl♦ 1 hour ago Undelete

@VisualMelon I don't think that would help - I see that as leading to answers specific to the particular usage, and not e.g. useful for a general library function. This seemed more suited to Code Review than Stack Overflow, but perhaps I was mistaken. Consider this question. – NetMage Jul 26 at 21:14 deleted by rolfl♦ 1 hour ago Undelete

Fair enough, but example code is still useful, not only so that we have some confidence the code has been tested, but also it's easy then for others to quickly get it up and running. I think your edits are good, but "My extension takes an array of string and a string to search and returns the lowest position of any one member of the array or -1 if none are found." isn't entirely clear. – VisualMelon Jul 26 at 21:19 deleted by rolfl♦ 1 hour ago Undelete

@VisualMelon I tried to re-write it clearer by naming things? – NetMage Jul 26 at 21:23 deleted by rolfl♦ 1 hour ago Undelete When trying IndexOfAny2() with this string "aabbccddeeffgghh" and these patterns { "bbb", "hh", "aa" } it returns -1. I think you'll have to reconsider this expression: curAns.HasValue ? curAns.Value + target.Length : s.Length and correct the question before we can give you a decent review. The idea of the expression is valid, but the expression is wrong :-) – Henrik Hansen Jul 27 at 5:47 deleted by Henrik Hansen Jul 27 at 6:03

When trying IndexOfAny2() with this string "aabbccddeeffgghh" and these patterns { "bbb", "hh", "aa" } it returns -1. I think you'll have to reconsider these expression: curAns.HasValue ? curAns.Value + target.Length : s.Length and if (!curAns.HasValue || posAns >= 0 && posAns < curAns) and correct the question before we can give you a decent review. The idea is valid, but the expressions is wrong :-) – Henrik Hansen Jul 27 at 6:04 deleted by Henrik Hansen Jul 27 at 6:25

When trying IndexOfAny2() with this string "aabbccddeeffgghh" and these patterns { "bbb", "hh", "aa" } it returns -1. I think you'll have to reconsider these expressions: curAns.HasValue ? curAns.Value + target.Length : s.Length and if (!curAns.HasValue || posAns >= 0 && posAns < curAns) and correct the question before we can give you a decent review. The idea is valid, but the expressions are wrong :-) – Henrik Hansen Jul 27 at 6:25 deleted by rolfl♦ 1 hour ago Undelete

Yeah, it's broken, "foo bar baz".IndexOfAny2("barz", "baz") returns -1. – t3chb0t Jul 27 at 18:03 deleted by rolfl♦ 1 hour ago Undelete

@t3chb0t @HenrikHansen I fixed the bug in IndexOfAny2. Thanks. – NetMage yesterday deleted by rolfl♦ 1 hour ago Undelete

I don't want to add oil to the fire, but if you would have added unit tests to the question, most of the aforementioned comments could have been prevented. Now, each reviewer is writing his own unit tests to perform some quick checks. – dfhwze yesterday deleted by rolfl♦ 1 hour ago Undelete

@dfhwze A fair comment, but my unit tests wouldn't have been that useful, since when I tested I didn't find any errors, so actually having others generate their own tests is better for debugging. And I think it is probably true in general that writing tests for your own code is not as useful as having others provide them. But your comment did make me add a comparison of IndexAny1 to IndexAny2 to my timing harness. – NetMage yesterday deleted by rolfl♦ 1 hour ago Undelete

@dfhwze I added test code from the comments. – NetMage yesterday deleted by rolfl♦ 1 hour ago Undelete

@VisualMelon I added a test harness I was using for timing. – NetMage yesterday deleted by rolfl♦ 1 hour ago Undelete You should also define what you mean by "best". I can think of at least threr different scenarios: fastest for one-time use, or fastest for testing many strings for the same constant occurrences, or terse code. – Roland Illig yesterday deleted by rolfl♦ 1 hour ago Undelete

@RolandIllig I never say "best" but I do say "or another way to make this faster" – NetMage 23 hours ago deleted by rolfl♦ 1 hour ago
Undelete

Never say never. The first word in the title of this question is "Best". – Roland Illig 23 hours ago deleted by rolfl♦ 1 hour ago
Undelete

@RolandIllig Nice catch. Now I wonder if I mean fastest, or most efficient (and would that necessarily be fastest?). – NetMage 23 hours ago deleted by rolfl♦ 1 hour ago Undelete

You must not edit the code when there are already answers. It'll invalidate them. I have rolledback your last edit. – t3chb0t 5 hours ago deleted by rolfl♦ 1 hour ago Undelete

@t3chb0t So then I never update the version in the question, but just leave the bugs found and improvements left undone? – NetMage 5 hours ago deleted by rolfl♦ 1 hour ago Undelete

You should leave bugs, yes :) Once you feel you have sufficient info from reviews, you could always ask a follow-up question with a new and updated version of your code. – dfhwze 5 hours ago deleted by rolfl♦ 1 hour ago Undelete

@dfhwze Even though the question would be identical to this one, just with corrected code? On the one hand, I am not particularly interested in microoptimizations but alternative approaches, but on the other hand, I don't see any point in new readers trying to answer, testing the code and running into the same bugs. On the gripping hand, perhaps the best compromise is to add another postscript to the question. – NetMage 4 hours ago deleted by rolfl♦ 1 hour ago Undelete

Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. – Simon Forsberg 2 hours ago

@SimonForsberg What happened to the previous comments? – NetMage 59 mins ago deleted by NetMage 58 mins ago

\$\endgroup\$
3
  • \$\begingroup\$ The code is the wrong code from before the bug fixes. It still has bugs in it. The comments by e.g. HenrikHansen and t3chb0t are fixes that are not in the question's code now due to rollbacks and the comments showing the bugs are also removed. \$\endgroup\$
    – NetMage
    Jul 30, 2019 at 21:36
  • 3
    \$\begingroup\$ @NetMage - that may be the case, in which case, you, or anyone else, can post an answer with the whatever's in the comments edited in to address the bugs/issues, and there's nothing stopping you from adding another question with the fully working code in it, for review for finer details. \$\endgroup\$
    – rolfl
    Jul 30, 2019 at 21:40
  • \$\begingroup\$ @I can, since I have the fixed code, and you provided the comments here. No one else can, because they can't see the necessary comments. I have posted an additional follow-up question as the least objectionable way to try again. \$\endgroup\$
    – NetMage
    Jul 30, 2019 at 21:43

You must log in to answer this question.

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