This is perfectly valid code. You cannot modify a collection which is being iterated safely so it's the best way to handle the situation.
[deleted]
What's wrong with this code, really?
111–120 of 217 posts
Re: What's wrong with this code, really?
#112Earlier quoted context omitted.
Depends on what you want to accomplish and what your container and iterators support. If your Iterator offers a delete method, I think it's a very elegant and clear way to filter a container. for (Iterator it = container.iterator(); it.hasNext(); ) if (!predicate(it.next())( it.remove(); It's more ugly and error prone if you've got to juggle an index, though.
Even with plain C, you can just iterate backwards: for (i = ctr_size(container); i > 0; i--) if (!predicate(container, i - 1)) ctr_remove(container, i - 1);
for (i = ctr_size(container); i--; )
if (!predicate(container, i))
ctr_remove(container, i);Re: What's wrong with this code, really?
#113What's wrong with the code, then, is that it was written under a little too much pressure for the developer to think clearly. for ( int i=0 ; i
Why not just: while (this.MyControl.TabPages.Count > 0) { this.MyControl.TabPages.Remove ( 0 ); }
Indeed. But the point I was making was that this hypothetical coder introduced more complexity by adding "i--;" whereas the code could equally well be fixed, in the same number of keystrokes, while reducing complexity, by removing "i++;".
Re: What's wrong with this code, really?
#114If the code compiles and does what it is supposed to do, then the answer is that nothing is 'wrong' with it.
Writing code that does what it is supposed to do is often not the challenge of software engineering - but writing code that can be easily tested, refactored, altered and ultimately understood by other developers is the harder part.
The conditional statement used in the 'for' loop whose value can not easily be determined is not helpful and the i--; is 'unusual'.
In any case, it is more useful to code review the unit tests than the code itself.
Re: What's wrong with this code, really?
#115Earlier quoted context omitted.
Even with plain C, you can just iterate backwards: for (i = ctr_size(container); i > 0; i--) if (!predicate(container, i - 1)) ctr_remove(container, i - 1);
for (i = ctr_size(container); i--; ) if (!predicate(container, i)) ctr_remove(container, i);
That's arguably better. Nitpick: start at ctr_size(container) - 1. [Feel free to edit your post, and I'll just delete this one.]
Re: What's wrong with this code, really?
#116This is perfectly valid code. You cannot modify a collection which is being iterated safely so it's the best way to handle the situation.
The code works, the problem is that it's not very obvious why it's going about it the way it does. You look at it and right away ask yourself "wtf, did I miss something?" simply because it's so unusual.
You might look at it for 2 minutes and figure it out, but that's 2 minutes too long for what's actually being accomplished. The problem is, if you were to come back and look at it again 6 months from now, it would take another 2 minutes.
The point of the article isn't really the method that was used to get to the result, but rather the fact that code should be made easy to read and understand, because +60% of the time is spent maintaining it. I usually tell this to newbie programmers, "code is meant for people to read, machines understand on/off".
Even if you need to borrow such a convoluted approach to clear a collection (as opposed to the more direct clear() method), there are simpler and more readable alternatives:
while(Pages.count > 0){
Pages.Remove(Pages[0]);
}Re: What's wrong with this code, really?
#117Earlier quoted context omitted.
I think there's a fundamental concept that isn't taught very well, and that concept is: Computers only ever do (for something like 99.999999999% of instructions) exactly what they are told to do. They don't have a mind of their own, and programming isn't magic. Opaque languages, libraries and APIs don't help the situation either. I wonder how many programmers start out under the assumption that computers are more-or-…
That summarizes the familiar high-level world of recent times. (But then I'm also old enough to remember when compiler gremlins were ubiquitous.) The lower-level world can be quite a bit more unseemly, what with the unpredictability of occasionally marginal voltages or power supplies, of RAS recovery and the occasional RAS and ECC errors, of sections of system buses lacking ED/EDC protection, the "fun" that is radioa…
And that's why I say that it works for the vast majority of instructions that are executed. There is that slim chance that it will flub up, but that kind of error either: A) gets caught in the hardware or B) completely takes down the operating system or app.
The likelihood of some cosmic ray flipping a bit of a counter variable in some loop's sub-millisecond lifespan is just so unimaginably small you can count it impossible.
Re: What's wrong with this code, really?
#118Earlier quoted context omitted.
Even with plain C, you can just iterate backwards: for (i = ctr_size(container); i > 0; i--) if (!predicate(container, i - 1)) ctr_remove(container, i - 1);
for (i = ctr_size(container); i--; ) if (!predicate(container, i)) ctr_remove(container, i);
filter predicate xs
Re: What's wrong with this code, really?
#119Earlier quoted context omitted.
Actually, you will never delete the final element with that code. I think what you meant was: for (int i=this.MyControl.TabPages.Count; i > 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i-1]); }
I prefer: for (int i=this.MyControl.TabPages.Count - 1; i >= 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i]); } Though a simple while loop is much easier to follow, even if its less efficient than removing the elements in reverse.
Exactly. For example in JavaScript doing a something.length would result in re-counting the number of elements, while doing the loop in this style would efficiently store the count in a variable that is fast to access and manipulate. I suppose if you want to be a real optimization junky you'd also use --i instead of i--.
Re: What's wrong with this code, really?
#120The article's point about writing code that does what it says it does is fine. But as an interview question, I have trouble imagining that candidates won't figure out that this is a game of "guess the answer I'm looking for" and say that they would rewrite this code. A better question would be, there's tremendous deadline pressure, the company is in imminent danger of losing a giant deal if we don't have working code…
Most do say exactly that, then we have a chat about what they might do instead. Many immediately say, "um, is there a Clear or RemoveAll method?!" It's all over within a few minutes, we move on. The bigger picture is always going to trump the details, until the day the details have piled up and can no longer be ignored. It's the great big technical-debt elephant in the room.
I think saying "yes, there's a Clear, but what would you do if there weren't", or "yes, there's a Clear - how would you implement it if you made the library?". Interview questions where the right answer is "I know an API or assume something about it correctly" are stupid, but questions where that is the right answer with a more interesting follow-up are not.