Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

What's wrong with this code, really?

cvmountain.com

Post reply on HN
Print view View original

91–100 of 217 posts

Re: What's wrong with this code, really?

#91

Post by lukeschlather »

microtonal wrote: post #47
simonsarris wrote: post #37

Earlier quoted context omitted.

Just for the record, your method is inefficient for a lot of implementations of lists/arrays, where it is often far faster to remove from the end than to remove from the beginning.

But we are talking about tabs here. Premature optimization is the root of all evil (to throw another cliché).

Premature optimization is the root of all evil, but removing from the beginning of any array-based growable structure rather than the beginning will turn a O(n) operation into an O(n^2) operation.

And you don't know what's going into those tabs. That cliché shouldn't be used to excuse positively brain-dead choices.

Re: What's wrong with this code, really?

#92

Post by tlrobinson »

masklinn wrote: post #40
AlexandrB wrote: post #21

Earlier quoted context omitted.

Why not just: while (this.MyControl.TabPages.Count > 0) { this.MyControl.TabPages.Remove ( 0 ); }

Very inefficient on most array lists (but a very good idea on linked lists)

  while (this.MyControl.TabPages.Count > 0)
  {
    this.MyControl.TabPages.Remove ( this.MyControl.TabPages.Count-1 );
  }

Re: What's wrong with this code, really?

#94

Post by simonsarris »

thaumaturgy wrote: post #78
simonsarris wrote: post #63

Earlier quoted context omitted.

Its heavily implementation dependent, but for many implementations of lists and arrays shift is slower than pop. Shift tends to require moving the entire array around, while pop does not. It's rarely the case with doubly-linked lists, or with perl arrays (they do something special, keep the starting offset recorded or something) For you case deleting from either end ought to be fine, but you've made the other implici…

Yikes. That's quite a difference in performance ... what I still don't understand though is, why? I've done a little bit of searching and I can't find anything so far on how JS arrays are implemented. Since JS arrays are objects and inherently support things like out-of-order indexes, non-integer indexes and that kind of thing, maybe we can assume it's some kind of hash map? A tree would make sense too, for faster ac…

Most of the time it is simply because a shift operation has to re-address each element in the array (if it is implemented like a ho-hum, classic array) and a pop operation does not have to do this.

Re: What's wrong with this code, really?

#95

Post by kzrdude »

jbri wrote: post #23
shabble wrote: post #9

Earlier quoted context omitted.

The other nice thing about running it in reverse is that the .count method (assuming it's a method and not a property), needs only to be accessed in the initial condition setup. If you're accessing the size of, say, a linked list, you end up sneaking an O(n^2) runtime because it has to re-count the size of the list every iteration to check for termination. c.f. https://secure.wikimedia.org/wikipedia/en/wiki/Schlemiel…

Though you have to be careful with that, because the obvious code for removing from a collection in reverse: for(i = list.size; i >= 0; i--) list.removeAt(i); Is itself a schlemiel algorithm on any singly-linked list...

You're removing i+1 elements here :)

Re: What's wrong with this code, really?

#96

Post by ajross »

raganwald wrote: post #12

The 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…

Surely the right answer would be something to the effect of "If this code survived in the source base until deployment, all is lost anyway."

The point of code review is to catch monsters like this when they are written. If that doesn't happen, what's the point?

Re: What's wrong with this code, really?

#98

Post by [deleted] »

thaumaturgy wrote: post #78
simonsarris wrote: post #63

Earlier quoted context omitted.

Its heavily implementation dependent, but for many implementations of lists and arrays shift is slower than pop. Shift tends to require moving the entire array around, while pop does not. It's rarely the case with doubly-linked lists, or with perl arrays (they do something special, keep the starting offset recorded or something) For you case deleting from either end ought to be fine, but you've made the other implici…

Yikes. That's quite a difference in performance ... what I still don't understand though is, why? I've done a little bit of searching and I can't find anything so far on how JS arrays are implemented. Since JS arrays are objects and inherently support things like out-of-order indexes, non-integer indexes and that kind of thing, maybe we can assume it's some kind of hash map? A tree would make sense too, for faster ac…

[deleted]

Re: What's wrong with this code, really?

#99

Post by Tyrannosaurs »

ajross wrote: post #96
raganwald wrote: post #12

The 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…

Surely the right answer would be something to the effect of "If this code survived in the source base until deployment, all is lost anyway." The point of code review is to catch monsters like this when they are written. If that doesn't happen, what's the point?

That's a very purist view of the world.

This code works and, as he says, was produced by a coder under time pressure. You show me a system and pretty much I'll show you a system that has poor (but working) code in it produced by a competent developer under pressure.

And code review is a useful process but it's no guarantee that issues will be caught any more than system testing or user acceptance testing.

And yet the world keeps on spinning and all is not lost.

I agree, you'd hope this was caught, but if someone gave that answer in an interview even aside from the tone, I'd wonder how much real world experience they had.

Re: What's wrong with this code, really?

#100

Post by hasslblad »

roel_v wrote: post #66
hasslblad wrote: post #17

As soon as I saw that snippet I could see what's wrong. In C# / .Net you can't remove an element from an enumerator while you're enumerating through it. You can remove the last element however, as it's the final loop the enumerator isn't used again so it won't throw an error. The original developer probably tried to remove it forward only first, encountered an error and wrote the code to loop through it backwards, us…

Yeah well except that that's not even related to what's happening here. Good thing though that you spotted it immediately and that you're smarter than all the developers you encounter ^_^

Well, when working on a code base, the first thing I think is who wrote this and when. When they wrote this how experienced was the developer? What's there coding style or was it written after coding standards were introduced? I've had this misfortune of inheriting a lot of bad legacy code in the past and when you see a chunk of bad code, you can either spend a good chunk of time figuring what it's really doing or less time figuring out the intent. Depending on who wrote the code, what it does, and various other factors you can decide which path to take.

With this code snippet it's obvious that the intent is to remove all the elements. So it can be fixed with a .Clear();

From a higher level, you can see that the snippet smells bad, and will need some attention.

I didn't say I'm smarter than all the developers I encounter. I've just encountered a lot of bad developers in my time.

Post reply on HN