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

51–60 of 217 posts

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

#51

Post by onemoreact »

RyanMcGreal wrote: post #32
MortenK wrote: post #27

About the McConnell quote: "Inefficient programmers tend to experiment randomly until they find a combination that seems to work." The essence of this quote is being passed around quite often these days. When you first start programming, you generally have no idea what the hell you are doing. You learn all these strange, abstract concepts best, by experimenting. It's easy to dismiss people "jiggling things around unt…

Understood, but I would caution that even an inexperienced, inefficient programmer should still understand what their code does and why it works . The best way the more experienced can exercise patience is by giving inexperienced programmers enough time to figure it out rather than pressuring them to flip random switches frantically.

With a little time, inexperienced and inefficent programmers can understand often understand the crappy code they wrote just fine. What they can't do is avoid writting it in the first place. For most of them if you say great this works now make it better they will have zero understanding in what direction they should try and improve things.

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

#52

Post by hasslblad »

AlexandrB wrote: post #21
qntm wrote: post #18

What'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 ); }

Or Even this.MyControl.TabPages.Clear();

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

#53

Post by tomjen3 »

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)

How many tabs do you use?

Less than 50?

Then don't worry about the optimization until you have to port it to a PDP10.

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

#54

Post by bad_user »

palish wrote: post #49
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.

Yep, that's correct. I wish "thing.Length - 1" were as clear as "0", but it's not. Besides, if you really cared about efficiency, you wouldn't be using growable containers... =)

There's usually a Remove() with no arguments that depending on the data-structure involved will remove from either the beginning or the end (depending on how it is more efficient for said data structure).

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

#55

Post by dkersten »

kevinkemp wrote: post #48
billybob wrote: post #34

Earlier quoted context omitted.

Correct. I think the logical thing to do would be to delete from the end so that the index is always valid. for (int i=this.MyControl.TabPages.Count - 1; i > 0; i--) { this.MyControl.TabPages.Remove(this.MyControl.TabPages[i]); } Imagine the count is 2. The first iteration you delete item 1, the second you delete item 0, and then the loop exits. EDIT: Actually, as someone else pointed out, it's clearer to use a while…

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.

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

#56

Post by jcromartie »

benhoyt wrote: post #25
altrego99 wrote: post #20

Earlier quoted context omitted.

Isn't clear() itself a "while (not empty) delete element"?

Very unlikely. There class implementation can almost certainly do it with an O-1 operation, like "self._internal_count = 0".

or self._tabs = new List();

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

#57

Post by thaumaturgy »

simonsarris wrote: post #35
brlewis wrote: post #31

Earlier quoted context omitted.

The problem is that .Count decrements every time you .Remove(). Either change [i] to [0] in your code, or initialize i with .Count-1 and decrement in the loop.

For the record, the second way is typically better: It is usually more (sometimes much more) efficient to remove from the end of a list/array than to remove from the beginning.

I'm genuinely curious -- why? The last time I coded my own linked-list, it was doubly-linked, so deleting either the head or the tail was exactly the same.

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

#58

Post by jcromartie »

MortenK wrote: post #27

About the McConnell quote: "Inefficient programmers tend to experiment randomly until they find a combination that seems to work." The essence of this quote is being passed around quite often these days. When you first start programming, you generally have no idea what the hell you are doing. You learn all these strange, abstract concepts best, by experimenting. It's easy to dismiss people "jiggling things around unt…

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-less magic?

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

#60

Post by kevinkemp »

dkersten wrote: post #55
kevinkemp wrote: post #48

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

Although your point on efficiency stands (at least with data structures that have to reshuffle contents on deletion), the type of loop that you use has nothing to do with the order that you delete the elements. You could easily do something like:

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

For loop are nothing more than while loops with:

(1) an assignment (int i = MyControl.TabPages.Count in this case)

(2) an extra command (i-- in this case) added to the end

Post reply on HN