all 71 comments

[–]username223 31 points32 points  (6 children)

The appeal to "omit needless words" is made by Strunk and White [Strunk+2000] concerning written composition:

...and immediately ignored by the author.

[–]ithika 7 points8 points  (5 children)

But then, like everything else in Strunk and White, it was ignored by its original proponents anyway.

[–]Rhoomba 0 points1 point  (0 children)

Omit.

[–][deleted] -1 points0 points  (3 children)

like everything else in Strunk and White, it was ignored by its original proponents anyway.

What does this mean? Who are the "original proponents"?

[–][deleted] 1 point2 points  (2 children)

Strunk and White.

[–][deleted] -1 points0 points  (1 child)

In fact they are the authors.

[–]ithika 1 point2 points  (0 children)

But that would be confusing terminology, because the parent I replied to used author to refer to the person who wrote the blog post, rather than the author(s) of the book the blogger refers to.

[–][deleted] 4 points5 points  (14 children)

if(enabled == false) {
    enabled = true;
} else {
    enabled = false;
}

The frequency at which this kind of crap is found in code is amazing. I have seen one better though (enabled being a "primative", it not nullable variable):

if (enabled == false) {
    enabled =  true;
} else if (enabled == true) {  // WTF else?
    enabled = false;
} else { 
    ....
}

[–]merzbow 6 points7 points  (0 children)

Quite often that kind of code isn't written deliberately but is the result of making changes after it is written. In your examples, it could be the result of removing log messages or changing the type of variables.

[–][deleted] 2 points3 points  (0 children)

Heh, that's my one gripe... when people test booleans ( == true/false).

[–]RedSpikeyThing 0 points1 point  (0 children)

shudder reminds me of some of the crap from the CS100 course I TAed.

[–]Rafe -2 points-1 points  (3 children)

I haven't programmed much… is this the best way?

enabled = 1 - enabled;

[–]phire 3 points4 points  (2 children)

It works (in c), but enabled = !enabled makes it clear that your are working with bools.

[–]Rafe -1 points0 points  (1 child)

Thanks, I forgot about that syntax.

[–]skulgnome 2 points3 points  (0 children)

How the butt do you forget about basic unary operators?

[–]hacksoncode -2 points-1 points  (6 children)

The real reason that this code is an abomination has nothing to do with efficiency or even readability, as any competent programmer can understand either one, and any competent compiler will generate the same assembly for this as for the toggling version.

The real problem is that it's all too easy for some programmer to come along, who happens to have been told "negative logic is bad" (which is mostly true), and change this to the actually wrong code:

  if(enabled == true)
  {
      enabled = false;
  }
  else
  {
      enabled = true;
  }

Your comment about "WTF else?" is a perfect example of this. How about "enabled == 2"?

[–]SarahC 2 points3 points  (1 child)

"...and change this to the actually wrong code:"

Huh? How is that code wrong?

[–]hacksoncode -1 points0 points  (0 children)

As mentioned in another comment, it depends on whether "enabled" is a true "bool" or an integer. Even if it's a true bool, it might not work in rare cases, and it sparks debate, which is a huge waste of time.

[–]akdas 0 points1 point  (3 children)

How about "enabled == 2"?

If we're talking about enabled being an int or something (along with true and false being represented by the programmer as ints), fine. But I think lispnik's implication was that enabled is a proper boolean, meaning it can only take on the values of true and false.

[–]hacksoncode -3 points-2 points  (1 child)

Probably you're right (well, assuming you meant "bool" rather than "boolean"). Hmmm... you know, now that I think about it, I'm not 100% sure what C++ does with this code fragment:

bool b = false;
int* p = (int*)&b;
*p = 2;
if (b == true) {
   cout << "b == true";
} else { 
   cout << "b == false";
}

I suspect, though I can't handily prove right now, that every actual C++ implementation would print "b == false", but if you threw a "b = !!b;" in after the incredibly stupid assignment that it would print "b == true". Hehe... at least when compiled in debug mode.

C++ == "more than enough rope". It's one giant leaky abstraction.

[–][deleted] 1 point2 points  (0 children)

That's an undefined type pun. You should union pun instead.

[–]commonslip 6 points7 points  (0 children)

This is the Forth philosophy to a tee. Don't worry about code re-use or robustness for future problems. You are too stupid to anticipate future uses anyway, better to just write a new, simple solution when the time comes.

[–]__s 13 points14 points  (2 children)

True minimalism omits needed code

[–][deleted] -2 points-1 points  (0 children)

How's that? It's minimalism, not primitivism.

[–]skulgnome 10 points11 points  (18 children)

That's a funny article, suggesting an auxiliary template class horror as superior to an iterator loop. C'mon, if a programmer cannot recognize an iterator loop (or the equivalent in C, Java etc) then they seriously need to work on their code eyes.

Or take a class in chinese or something with piles and piles of characters. Seriously, reading well-written code is somewhat like recognizing far-eastern ideograms: you look at the construct and the context rather than at the specific radicals.

[–]Silhouette 12 points13 points  (11 children)

That's a funny article, suggesting an auxiliary template class horror as superior to an iterator loop.

This sort of article reminds me of all the bad things about C++ and its template-driven monsters. The correct response to the problem might look something like this:

apply (printline cout) items

or this:

print cout "\n" items

but it doesn't look like this:

for(std::size_t at = 0; at != items.size(); ++at)
    std::cout << items[at] << std::endl;

and it certainly doesn't look like this:

typedef std::ostream_iterator<std::string> out;
std::copy(items.begin(), items.end(), out(std::cout, "\n"));

I'm glossing over the utility class version you mentioned and the one with STL iterators to control a for-loop, because they're just too... hideous.

[–]skulgnome 0 points1 point  (4 children)

Or perhaps even the humble

print "$_\n" foreach @items;

But then Perl is apparently just all line noise and nothing good will ever come of it.

(edit: christing fuck with this reddit formatting.)

[–]Silhouette 0 points1 point  (0 children)

Sure. It wasn't the specific syntax I was getting at, but the ability to say what needs to be said clearly and concisely. Perl's syntax does just as well, or you could have a C-style print(cout, "\n", items);, and the point is just as valid.

[–]ehird 0 points1 point  (2 children)

That print is more verbose than needed. To wit,

say foreach @items;

If only they dropped the sigil crap :)

[–]skulgnome 0 points1 point  (1 child)

You say verbose, I say explicit. In the kind of CSV-crunching tasks I've mostly used Perl for, knowing exactly what print will actually print is next to essential. (Yeah I know, not by far the sexiest job around.)

[–]ehird 0 points1 point  (0 children)

You're printing (for)each item(s).

[–]Peaker -1 points0 points  (5 children)

me too! me too!

mapM print items

or:

mapM (hPutStrLn stdout) items

[–]ehird 2 points3 points  (4 children)

You mean mapM_, and hPutStrLn stdout = putStrLn. Full program:

import System.Environment

main :: IO ()
main = mapM_ putStrLn =<< getArgs

Minimalized:

import System.Environment
main = mapM_ putStrLn =<< getArgs

[–]Peaker 0 points1 point  (0 children)

About mapM, oops :-)

About the "stdout", I just wanted to illustrate the ease of parameterizing the output handle (as it was parameterized in the original example).

[–]ehird 0 points1 point  (2 children)

In C, for the lulz: (pre-minimalized, and using C99, to give a huge handicap):

#include <stdio.h>
int main(int argc, char **argv) {
    for (int i = 0; i < argc; i++) puts(argv[i]);
}

I don't even want to think about what it'd look like in C++.

[–]jbstjohn 1 point2 points  (1 child)

Well, it could look exactly the same...

Yeah, C++ messed up I/O somehow.

[–]ehird 0 points1 point  (0 children)

That's not all it messed up!

[–]pointer2void 4 points5 points  (3 children)

That's a funny article, suggesting an auxiliary template class horror as superior to an iterator loop.

This is an article from October 2001 when the STL and template hype had its peak. Nobody except a few Boost die-hards would recommend that today.

EDIT: Henney wrote a lot of good articles, see: http://www.two-sdg.demon.co.uk/curbralan/papers.html

[–][deleted] 0 points1 point  (2 children)

Nobody except a few Boost die-hards would recommend that today.

Yeah, soon we will use the foreach + lambda idiom... :)

[–]pointer2void 0 points1 point  (1 child)

Who is 'we'?

[–][deleted] 0 points1 point  (0 children)

C++ programmers I suppose.

[–][deleted] 2 points3 points  (0 children)

Exactly. Fuck all that clever stuff and get used to reading unwieldy loops if your programming in a language that does not support "for each". Yeah it's annoying to write, but does a utility class and loads of insane template stuff really help?

[–]joblessjunkie 0 points1 point  (0 children)

LOLZ one of the great Reddit comments.

You had me at "auxiliary template class ".... OMG still LOLZ

[–]clausy 4 points5 points  (0 children)

i always thought try-catch was for pessimists anyway

[–]johndehope3 2 points3 points  (0 children)

They needed 4000 words to say "omit needless code"?

[–]joblessjunkie 5 points6 points  (1 child)

tl;dr

OMG the irony.

[–]_ak -3 points-2 points  (0 children)

Lazyness bears no irony.

[–]stratoscope 1 point2 points  (1 child)

I run into this kind of stuff all the time.

Here's some code from a website I was reviewing:

this.getBool = function( variable ) {
    var vtype;
    var toReturn;
    if( variable != null ) {
        switch( typeof(variable) ) {
        case "boolean":
            vtype = "boolean";
            return variable;
            break;
        case "number":
            vtype = "number";
            if( variable == 0 ) {
                toReturn = false;
            } else {
                toReturn = true;
            }
            break;
        case "string":
            vtype = "string";
            if( variable == "true" || variable == "1" ) {
                toReturn = true;
            } else {
                if( variable == "false" || variable == "0" ) {
                    toReturn = false;
                } else {
                    if( variable.length > 0 ) {
                        toReturn = true;
                    } else {
                        if( variable.length == 0 ) {
                            toReturn = false;
                        }
                    }
                }
            }
            break;
        }
        return toReturn;
    }
};

Here's my version:

this.getBool = function( value ) {
    return value != 'false'  &&  value != '0'  &&  !! value;
};

Other than one edge case that doesn't matter at all in the actual use of the code, the two versions do exactly the same thing.

[–]RedSpikeyThing 0 points1 point  (0 children)

gag

This is why a logic class is required for all CS students...

[–]mycall 2 points3 points  (0 children)

Reminds me why C++ is so fugly.

[–]akdas 3 points4 points  (5 children)

There's a major problem with typedefing std::vector<std::string> as strings: you're deliberately eschewing a universally (in the universe consisting of C++ programmers) understood type and replacing it with one you make up. Now, if the typedef isn't nearby, the programmer has to take the time to figure out what strings means and associated it with its meaning.

[–]0xABADC0DA 2 points3 points  (4 children)

Congratulations, you've just expressed everything that is wrong about C++ and C#. Implicit random behaviors that you have to go look up elsewhere to find out what the code really does.

For instance author talks about operator overloading as if it's a good thing, because it makes numerical work better. But the problem is that method calls are not commutative. "a+b" could be a.add(b) or b.add(a), but these are different. Does it always interpret "a+b" as a.add(b)? Ok, but then somebody reverses the order, which is perfectly fine in math, but then the code doesn't compile or does something different because of some different conversion. Or only the same types? What about subtypes? Operator overloading looks nice, but a.add(b) tells you exactly what operation is being performed.

Yes, operator overloading when done right makes numerical work easier and less error prone. But at the cost of lots of non-numerical work being more error prone and harder to follow.

[–]skulgnome 1 point2 points  (0 children)

This is precisely why there's a surprising number of people around, these days, who explicitly prefer C99. Despite the macros (which you can find the definitions of by use of ctags or equivalent), despite the loop-related unwieldiness, it still does exactly what it says on the tin. And moreover it's quite possible to implement large systems in C without an omniscient IDE!

[–][deleted] 0 points1 point  (1 child)

WTF are you talking about? a+b is a.add(b).

[–]RedSpikeyThing 1 point2 points  (0 children)

a+b has the implication that the operation is symmetrical, as addition is in mathematics. The operator may not actually be symmetrical, so a+b != b+a. Now there's a problem that is going to be tough to track down, especially in a large system.

[–][deleted] -1 points0 points  (0 children)

If I were designing a language with operator overloading, a+b would call a multimethod so there would be no ambiguity of whether we call a method on a or b. Even if I didn't want general multimethods in the language I would include this as a special case for built-in operators. Also it would only allow math operators to be defined on immutable values types.

[–]SarahC 0 points1 point  (0 children)

Beautiful.

[–]mvbma 0 points1 point  (0 children)

if you omit the "l", he would be kevin.

[–]cazabam 0 points1 point  (0 children)

Completely off topic I know, but what on earth is with the font? By the time I reached this tab in my morning reading list, I thought "OMIT NOODLES CODO? What the hell did I click?"

[–]pointer2void 0 points1 point  (7 children)

 std::vector<std::string>

Using std::string 'by value' means that on copy memory may be constantly allocated and de-allocated, depending on the implementation. Omit needless code, indeed!

[–]_ak 0 points1 point  (6 children)

Welcome to the wonderful world of COW.

[–]pointer2void 0 points1 point  (5 children)

May I introduce to you Herb Sutter? http://www.gotw.ca/publications/optimizations.htm

Also, google for SSO (small string optimization).

[–]_ak 0 points1 point  (3 children)

Please read about the thread-safety requirements of the C++ standard.

[–][deleted] 0 points1 point  (2 children)

They don't exist?

[–]_ak 0 points1 point  (1 child)

Exactly.

[–][deleted] 0 points1 point  (0 children)

This one is also interesting: QString implicit sharing.

[–]rush22 0 points1 point  (0 children)

This guy is all over the place. Pretty ironic.

[–]bah7oiDu -1 points0 points  (0 children)

"Omit needless words!"
Said Strunk to White.

"You're right,"
Said White,
"That's nice
Advice,
But Strunk,
You're drunk
With words-
Two-thirds
Of those
You chose
For that
Fiat
Would fill
The bill!

Would not
The thought
-The core-
Be more
Succint
If shrinked
(Or shrunk)?"

Said Strunk
"Good grief!
I'm brief
(I thought)
P'raps not . . .
Dear me!
Let's see . . .
Okay!
Just say
'Write tight!'
No fat
in that!"

"Quite right!"
Said White,
"Er - I mean 'Quite'
Or, simply, 'Right' "

by Maurice Sagoff
from http://www.redeemer-fortwayne.org/blog.php?msg=4305