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

101–110 of 217 posts

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

#101

Post by boredguy8 »

jcromartie wrote: post #58
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-…

Physical objects only ever do exactly what they're 'told' to do, period. Rocks don't have a mind of their own, and physics isn't magic. Yet I don't want to live in a world where we don't spend time twiddling with dials and poking around.

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

#102

Post by droz »

dean wrote: post #82
onemoreact wrote: post #50

That chart of development costs ignores the fact that only successful projects get maintained. Many projects simply get abandoned before they ever gain traction and at that point code quality becomes meaningless.

That's a good point. And if the project consists of mainly bad code, it probably contributes to the project being abandoned, as it gets harder and harder to fix bugs and add new features in a code base like that.

Most projects are abandoned due to political reasons. Very rarely do they get dismissed because someone didn't code to whatever standard of the moment.

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

#103

Post by KirinDave »

yardie wrote: post #4

Anyone else read it and on the first pass think, "yeah that works". Then on second pass think, "it's not a good idea, but it works." Then finally think, "under pressure I've done worse; at least this works as intended. S/he should probably comment it." Or is it only me? Addendum: I would also add that even as a junior program, Clear() was easily learned within the first few minutes and usually when you have to use a…

The code was unacceptable from line 1 onwards, and would not pass code review with me on watch.

For one, for loops have invariants.

Next up, you are altering the loop bound in the loop. I do not care if the code works, the code is hard to reason about when it goes all non-linear like that.

Finally, there is a goddamn Clear method.

That is the kind of code you see the next morning and delete; hoping none of your peers review the version control log that closely.

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

#104

Post by aphyr »

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.

True for arrays. Very wrong for singly-linked lists, as in lisp, haskell, erlang, etc.

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

#105

Post by Hoff »

jcromartie wrote: post #58
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-…

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 radioactive RAM and embedded alpha emitters (particularly "entertaining" if you don't have solid EDC), bus transients, and a host of other hardware gremlins.

Looking at the lowest levels of these boxes, I something marvel that these modern computing boxes even work at all, much less as well as they do.

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

#106

Post by tspiteri »

palish wrote: post #13
yardie wrote: post #4

Anyone else read it and on the first pass think, "yeah that works". Then on second pass think, "it's not a good idea, but it works." Then finally think, "under pressure I've done worse; at least this works as intended. S/he should probably comment it." Or is it only me? Addendum: I would also add that even as a junior program, Clear() was easily learned within the first few minutes and usually when you have to use a…

No, I pretty much just recoiled in horror immediately. Even if I had to avoid .Clear(), there are immediately-obvious, better ways. while ( !thing.Empty() ) thing.Remove( 0 ); In fact, the code isn't just bad; it's risky. What if someone changes "int" to "uint"? It wouldn't even infinite loop / crash... It would remove exactly one element!

What if someone changes "int" to "uint"? It wouldn't even infinite loop / crash... It would remove exactly one element!

I can't see that. If i is a uint, at the end of the first cycle, i-- changes i to the maximum uint, then i++ changes i to 0, then i 

  while ( 0

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

#107

Post by willfarrell »

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

for(i = list.size; i--;) list.removeAt(i);

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

#108

Post by [deleted] »

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é).

[deleted]

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

#109

Post by microtonal »

lukeschlather wrote: post #91
microtonal wrote: post #47

Earlier quoted context omitted.

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.

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.

I'll bite: in arrays, front deletion (like back deletion) can be done in O(1) time. Keep a pointer to the first element, and just increment it upon front deletion (and free the element if necessary). Obviously, you may want to be smart to avoid memory use getting out of hand.

And you don't know what's going into those tabs.

If the array stores references (pointers), which you'd expect with tabs, that is orthogonal to the discussion. Whatever is going into those tabs, a pointer is a pointer.

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

#110

Post by dcosson »

Great post - I can get pretty OCD about the way code is written, but I have a hard time complaining about things like this without feeling like a dick since as you pointed out it's not particularly inefficient (even if there was an O(1) Clear() method, how many TabPages are we really working with that it would matter?) But you've reassured me that it's a reasonable thing to do, especially if I know beforehand that it's a piece of code that will probably be used for a long time.

That said, I've learned that with an early stage startup where you're trying to iterate as quickly as possible in a desperate attempt to get somebody to care about your product, you often have to pick your battles. Just yesterday I came across this:

    category_count = []
    for i in range(10):
        category_count.append( db.execute("SELECT count(*) FROM table WHERE category = %d" % i) )
For one thing, this iterate separately and then append to list approach in Python annoys me slightly (list comprehensions are so much cooler!). But far worse, it hits the DB 10 times instead of once, and no matter how small your site is you obviously can't be having that. How'd it get there? Who knows. It was written in the Django ORM, where the only way to do this is with a pretty obscure command like Object.values('category').annotate(count=Count('category')). At first we were picking up Django as we went, so at the time whoever wrote it probably had no idea that the values() or annotate() methods even existed, and the way it was written got something up on the page and working so we could decide whether or not we'd be throwing it out the next week. But, whatever, you come across something like this, go throw up, fix it and move on. And finding these kinds of issues puts into perspective smaller ones like using a for loop where you meant to use a while loop.

tl;dr - Having the luxury of sexy-ing up your your code as described in the post is strongly dependent on the stage that the project/company is in.

Post reply on HN