The Daily WTF: Curious Perversions in Information Technology

AndrewB · 2005-02-17 Reply Admin

I don't like same-line braces. This is the 21st century. [:@]

JamesCurran · 2005-02-17 Reply Admin

wow.... There are 3 or 4 different misuses of try/catch there.....

Done sensibly:

try
{
    datReadr = dataCmd.ExecuteReader();
}
catch (DataNotFoundException dnfe)
{
        log.error(dnfe);
        throw new ManagedException(e.GetType().Name)
}
catch (Exception e)
{
       throw new DataNotFoundException("Exception while importing operational data.");
}

well, not entirely sensibly, since I duplicated the results of the code, which is pretty screwy to start with (transforming DataNotFoundExceptions in Managed Exceptions and everything else into DataNotFoundExceptions)

I'm sure when the drugs wear off, the author will realize what he really wanted all along was:

try
{
   datReadr = dataCmd.ExecuteReader();
}
catch (Exception e)
{
    log.error(e);
    throw;
}

2005-02-17 Reply Admin

JamesCurran:
wow.... There are 3 or 4 different misuses of try/catch there.....
Done sensibly:

I think the single biggest WTF in this sample is

<font size="4">try {
   throw e;
  } 
</font>

KraGiE · 2005-02-17 Reply Admin

:
[image] JamesCurran wrote:

wow.... There are 3 or 4 different misuses of try/catch there.....
Done sensibly:

I think the single biggest WTF in this sample is
try {
 throw e;
 }

I dunno man.. the very next line after that throws me off quite a bit.
} catch (Exception e2) {

If there's another catch.. is that E3?

Blue · 2005-02-17 Reply Admin

Another good name for this thread would have been "What's the catch?"

memorex · 2005-02-17 Reply Admin

Come on, come on.

Who hired this "C# developer"....?

He probably has 6 years in C# and .NET to be able to do something so odd...

2005-02-17 Reply Admin

What the heck is a Managed Exception anyway? I can't find it on MSDN...

znaps · 2005-02-17 Reply Admin

AndrewB:
I don't like same-line braces. This is the 21st century. Angry

znaps · 2005-02-17 Reply Admin

The forum software sucks donkey's balls.

I was trying to say he must be a Java programmer as Sun advocate same line braces.

Unforgiven · 2005-02-17 Reply Admin

The real WTF here is that he's catching Exception, something which you should never, ever do.

2005-02-17 Reply Admin

znaps:
I was trying to say he must be a Java programmer as Sun advocate same line braces.

I can't understand why same-line braces were not instantly abandoned for something actually readable as soon as 24-line terminals were no longer the normative development environment.

But this is a bit off-topic (said he after getting his shot in).

JamesCurran · 2005-02-17 Reply Admin

:

I can't understand why same-line braces were not instantly abandoned for something actually readable as soon as 24-line terminals were no longer the normative development environment.

As far as I can tell, UNIX programmers actually enjoy writing unreadable code. (I think it's a macho thing)

And 24-line terminals lasted much longer than you'd think --- Again, as far as I can tell, the biggest use of X-Windows was/is to open multiple 24x80 command prompts window (most running vi)

2005-02-17 Reply Admin

Same-line first brace is just as readable as the wasted-line first brace as long as you don't bork the indentation. Personally, I find the disconnect between block declaration and actual block code disorienting whenever I have to read code with an extra blank line for the brace.

Blue · 2005-02-17 Reply Admin

Re: "The real WTF here is that he's catching Exception, something which you should never, ever do."

Uhm, exactly why is that? Unless I'm going to do something different for a particular subclass of exception, simply catching Exception as a catch-all seems to work quite well for most cases I can think of. (And no, I don't ignore them ala catch {} (or "on error resume next" for you VBers)

Blue · 2005-02-17 Reply Admin

Brace placement is a matter of personal style / company coding standards (based on someone's personal style).

I challenge anyone to state any fact as to why one is better than the other.

That being said, I prefer not to use same-line bracing, and am careful to maintain proper indentation (which should be done regardless of brace style)

2005-02-17 Reply Admin

The Linux kernel coding style docs say to use the same-line curly brace with loops because that makes it harder to forget to put one on the next line. Remember that in C the language does not require curly braces on for loops if there is only one command in the loop.

Personally I've seen bugs caused by people who forgot a set of braces like this or who modified code to add a command to a loop but forgot the brace. (Perl says braces are always needed on loops and avoids this problem and the dangling-else problem.)

KraGiE · 2005-02-17 Reply Admin

:
What the heck is a Managed Exception anyway? I can't find it on MSDN...

He's most likely using the Exception Application Block. or he has custom exceptions. They're fairly common, and I've found great uses for them. Ain't like a VB programmer that wants to handle exceptions using a delegate. lol I still find that one funny.

KraGiE · 2005-02-17 Reply Admin

znaps:

[image] AndrewB wrote:

I don't like same-line braces. This is the 21st century. Angry

I love same-line braces. It annoys the hell out of me when I see a bunch of whitespace with just one { in it. it's like.. no shit sherlock, that's the start of the function?

What I abhor is seeing..

if (condition)
{
// one line of code;
}
else
{
// another line of code;
}

To me, that's just dirty. look at all that real estate wasted by 4 brackets. *shudder*

2005-02-17 Reply Admin

Says who/what? If your library is logging exceptions internally as the code - albeit wrong in other ways - is doing, it's perfectly reasonable to catch Exception and rethrow it if you don't plan on trying to recover from specific exceptions, assuming you even can. This allows additionally logging from your library while still allowing the caller to catch the exceptions and handle them however they require.

2005-02-17 Reply Admin

:
Says who/what? If your library is logging exceptions internally as the code - albeit wrong in other ways - is doing, it's perfectly reasonable to catch Exception and rethrow it if you don't plan on trying to recover from specific exceptions, assuming you even can. This allows additionally logging from your library while still allowing the caller to catch the exceptions and handle them however they require.

Oops, wrong button. This was in reference to the statement above "The real WTF here is that he's catching Exception, something which you should never, ever do."

JamesCurran · 2005-02-17 Reply Admin

KraGiE:
To me, that's just dirty. look at all that real estate wasted by 4 brackets. *shudder*

I'll let you in on a secret..... Whitespace is FREE (and infinite....)

JamesCurran · 2005-02-17 Reply Admin

:
The Linux kernel coding style docs say to use the same-line curly brace with loops because that makes it harder to forget to put one on the next line. Remember that in C the language does not require curly braces on for loops if there is only one command in the loop.

I'm baffled as to what they are saying there.... Are they trying to say that it's easier to spot the syntax error in

if (condition) {
do_stuff();

than in

        if (condition)
        {
              do_stuff();

If so, then Linux folks are as crazy as I always thought...

Stan Rogers · 2005-02-17 Reply Admin

Well, unless you're keeping your source files on your old TRS-80 Model I, where the extra characters can mean the difference between "fits in 4K" and "oh, crap....".

tonedef · 2005-02-17 Reply Admin

Blue:
Re: "The real WTF here is that he's catching Exception, something which you should never, ever do."

Uhm, exactly why is that? Unless I'm going to do something different for a particular subclass of exception, simply catching Exception as a catch-all seems to work quite well for most cases I can think of. (And no, I don't ignore them ala catch {} (or "on error resume next" for you VBers)

Although I agree with you Blue, I think the "something which you should never, ever do." comes from using FxCop as dogma. The new MS code standards really frown on this for some reason.

2005-02-17 Reply Admin

Just think what this guys code would look like if he knew about finally blocks.

2005-02-17 Reply Admin

(Ah, here we are again: the WTF forum software and me, locked in mortal combat. My cookies got blown away by something the helpdesk did yesterday, so I'm logged out. Squinting at the forum's UI, I finally find the line that says "Author: Anonymous". So then I get to pore over the screen trying to find the "login" button... Aha! It's the one in the top left, the 8x8 greyscale picture of a deformed baby with a big nose, or possibly a fish. Or maybe it's ripped off from MS Office - I'm sure I've seen it there before. Wouldn't it be delicious if it turned out to be the icon for MS Access...? Have to admit, these guys have a sense of humour, which sort of makes up for their inability to program. Sigh.)

2005-02-17 Reply Admin

Here's the obligatory "defense of the code" bit:

There's nothing wrong with this code. It works fine. Blah blah blah. Yadda yadda.

Thanks for listening.

Ralph

bat · 2005-02-17 Reply Admin

(I meant top right, actually. Haven't had my four litres of caffeine yet this morning.)

OK, I'm here now. Just wanted to say:

My preference is for this style:

void ploppy()
{
 if (fnord && sponn) {
 whackett();
 } else {
 plinge("foo", bar[baz]);
 }
}

That's just something I've grown used to. Function definitions deserve the extra spacing; ifs/whiles/fors/foreaches don't, so much.

That being said, if your function doesn't fit on one 80x25 screen, it seriously needs to be shortened¹, so your brace style is really about as relevant as the OS/2 port of Opera...

¹ Unless it's a big case/switch multiplexer of course, but There's An Exception To Every Rule (Except This One).

(BTW I hate the fact that Ctrl-Numpad-5 doesn't select all in this UI. I shouldn't complain, since Ctrl-A does. I guess part of my problem is that I want everything to be Emacs... And is that so unreasonable? Hmmm...)

2005-02-17 Reply Admin

Dear Ralph,

Thanks for sharing.

We appreciate your "defense of the code" and will take your comments under consideration.

-Mike

AndrewB · 2005-02-17 Reply Admin

JamesCurran:

[image] KraGiE wrote:

To me, that's just dirty. look at all that real estate wasted by 4 brackets. *shudder*

I'll let you in on a secret..... Whitespace is FREE (and infinite....)

Exactly. Readability is 10x as important as space-efficiency in your source files. The ability to understand what a program is doing is the #1 bottleneck for programmers.

"Wasted real estate"? Talk about a non-issue. Who cares if your source code is 7700 bytes instead of 7500 bytes?

AndrewB · 2005-02-17 Reply Admin

By the way, we all know that this is the REAL preferred way to eliminate errors in your program:

[code language="c#"]
void main()
{
   try
   {
      // Put program code here
   }
   catch (Exception e)
   {
   }
}
[/code]

There. Clean, efficient, and bug-free.

matejcik · 2005-02-17 Reply Admin

i -hate- the forum software, muhahaaa [6]

ad the "real estate" - the screen real estate, of course. honk if you want your editor to be covered by lines of basically nothing from top to bottom. no matter how many lines, easily half of them can be taken up in an instant

phx · 2005-02-17 Reply Admin

Ah the good old try-throw-catch construct. Not only is this stupid, but also immediately catching it is just plain dumb. The thing that makes it funnier is that theres no additional lines. Did throw immediately followed by catch not trigger off a few neurons in the author's brain?

Also:
e.GetType().Equals(typeof(DataNotFoundException))
e is DataNotFoundException

Easier to read? Yes.

Paul B · 2005-02-17 Reply Admin

tonedef:
[image] Blue wrote:
Re: "The real WTF here is that he's catching Exception, something which you should never, ever do."

Uhm, exactly why is that? Unless I'm going to do something different for a particular subclass of exception, simply catching Exception as a catch-all seems to work quite well for most cases I can think of. (And no, I don't ignore them ala catch {} (or "on error resume next" for you VBers)

Although I agree with you Blue, I think the "something which you should never, ever do." comes from using FxCop as dogma. The new MS code standards really frown on this for some reason.

Paul B · 2005-02-17 Reply Admin

The biggest WTF on this site is the forum software... Anyway..

Catching exceptions of type Exception is fine as long as you are rethrowing the exception or making an intelligent decision to wrap the exception. The big no-no is to make a concious decision to catch exceptions of type Exception and then swallow them. What if the framework throws an OutOfMemoryException, a StackOverflowException, or a ExecutingEngineException? You cannot rely on the framework to correctly execute code in finally blocks when these exceptions occur and therefore you cannot rely on your application to work as expected. Exceptions such as the ones I mentioned MUST be propagated to the top level exception handler or the CLR where the application can be closed; there is no other logical way to handle those sort of exceptions imho.

Applied Microsoft .NET Framework Programming by Jeffrey Richter has an excellent chapter on exception management. After reading if I felt that I had been irresponsible to be writing applications without the knowledge it gave me.

JamesCurran · 2005-02-17 Reply Admin

phx:
e.GetType().Equals(typeof(DataNotFoundException))
e is DataNotFoundException
Easier to read? Yes.

Yes, but not (exactly) the same thing.

e.GetType().Equals(typeof(DataNotFoundException)) match only DataNotFoundException objects.

e is DataNotFoundException matchs DataNotFoundException- derived classes also.

bat · 2005-02-17 Reply Admin

Incidentally, why is it

if (e.GetType.Equals(typeof(DataNotFoundException)))
{...}

instead of

if (typeof(e) == typeof(DataNotFoundException))
{...}

Surely typeof can handle polymorphism! I mean, if you really hate infix operators, why not just give up your petty, primitive Algol dialects and use LISP like real programmers do?

(cond
((eq (get-type e) 'DataNotFoundException)
...))

makes much more sense!

2005-02-17 Reply Admin

But screen space is not. :-/

2005-02-17 Reply Admin

Why is it bad to catch "Exception" and not do anything with it? Because this means that if a NullPointerException or an IndexOutOfBoundsException or a yadda yadda whatever exception happens inside your Try block, then the program will swallow it and you will never know that it happened.

Catching Exception is like saying "I don't care what errors you experience, program! Just ignore it! I don't want to know about it!"

LordHunter317 · 2005-02-17 Reply Admin

Catching of the generic, top-level 'Exception' class in any language almost always means there's an logic error in your code.

The only time it'd make any sense is if you had some sort of generic-catch all condition that had to be run on every exception. For 99% of applcations, the only thing that needs to run on every condition is are closures, and we already have a mechanism for that: the finally clause. Obviously, in a language without finally, you'd have little choice, but thankfully, that's not the typical case.

Seriously, what kind of other generic code can you run on all exceptions? You might be able to run a generic error message display thing to the user in a sufficently HLL (meaning not C#, not Java).

And since you can't reliably display a message that's friendly to the user in all cases, much better to let the exception propogate and just let the generic handler do the error reporting. At least it'll help you (even if it doesn't help them).

cjs · 2005-02-18 Reply Admin

I'll state why one is better than the other. It's all a matter of fonts, really. With some terminal fonts and line spacings, not having the brace on the same line makes things get cramped and hard to read. That is exactly why you should put them on the same line. If the lines are too cramped, you can always change fonts or just add a little more spacing between lines in your terminal window. However, you can't go the other way if you're already using a compact font with minimal line spacing.

Drak · 2005-02-18 Reply Admin

bat:
Incidentally, why is it

if (e.GetType.Equals(typeof(DataNotFoundException)))
{...} 

instead of

if (typeof(e) == typeof(DataNotFoundException))
{...}

Surely typeof can handle polymorphism! I mean, if you really hate infix operators, why not just give up your petty, primitive Algol dialects and use LISP like real programmers do?

(cond
((eq (get-type e) 'DataNotFoundException)
...))


makes much more sense!

In VB.NET I always use
If typeof ex is DataNotFoundException Then

Except... (guffaw) I never check my exception types that way because that's what multiple catches for one try are for. See... even VB programmers know a thing or two!

Drak

Jon Limjap · 2005-02-18 Reply Admin

Nice two-headed thread: Curly Brace Wars and Try Catch Baseball! [:P]

V. · 2005-02-18 Reply Admin

If we must discuss braces, I don't mind if it's

//my preference
public void SomeFunction(){
 if(somethingtrue){
 //do stuff here
 }
 else{
 //some other stuff here
 }
}

or

public void SomeFunction()
{
 if(somethingtrue)
 {
 //do stuff here
 }
 else
 {
 //some other stuff here
 }
}

What annoys me more is something like this

if(somethingtrue)
 //one line of code
else{
 //more lines of code
 if(somethingelsewouldbetrue){
 //lines, lines
 }
 else
 //1line again
}

and so one. Yes I know, It's correct to the compiler, but when you have a lot of nested ifs and elses and you have to change something... [8-)]

2005-02-18 Reply Admin

Hey this must be one of the most effective ways to flip exception types I've seen

try {
 datReadr = dataCmd.ExecuteReader();
} catch (Exception e) {
 if (e.GetType().Equals(typeof(DataNotFoundException))) {
...
 } else {
  throw new DataNotFoundException("Exception while importing operational data.");
 }

2005-02-18 Reply Admin

KraGiE:

I love same-line braces. It annoys the hell out of me when I see a bunch of whitespace with just one { in it. it's like.. no shit sherlock, that's the start of the function?

What I abhor is seeing..

if (condition)
{
   // one line of code;
}
else
{
   // another line of code;
}

To me, that's just dirty. look at all that real estate wasted by 4 brackets. *shudder*

You are right! I propose doing away with the braces. Let us end an if statement with "end if". Oh, and then we also get "end function" etc.

There, that'll be better. Hmm, we'll soon all be coding in Visual Basic, Yay!

2005-02-18 Reply Admin

As the poster of this code I just wanted to make a few follow-up comments:

Firstly, this code was originally written in Java so forget any C# implications (it was changed to protect the client but not sure why it was changed to C#). Also, the bracing style used is the Sun standard convention and happens to be preferred by many Java developers. Who really cares when modern IDEs allow you to reformat with one key combination?

Secondly, I don't think one reply has mentioned what I think is the worst part of this code: the switching of the caught exception type - that is, catching a DataNotFoundException and throwing a ManagedException or catching anything that is NOT a DataFoundException and throwing a DataNotFoundException.

I think the "throw e" immediately followed by "catch Exception" is the close runner up.

Anyway, this is how I would rewrite this (forgive my C# syntax, I'm a Java developer) - though I would usually not just catch Exception:

<FONT size=1>try {
 datReadr = dataCmd.ExecuteReader();
} catch (Exception e) {
 log.error(e);
 if (e.GetType().Equals(typeof(DataNotFoundException))) {
   throw (DataNotFoundException) e;
 } else {
   throw new ManagedException("Exception while importing operational data." + e);
 }
}   </FONT>

2005-02-18 Reply Admin

The reason braces on the same line are helpful is for code like this:

int i;

for(i=0; i<10; i++);
{
printf("guess what goes here: %d\n", i);
}

2005-02-18 Reply Admin

Whitespace is free if your time has no value ...

2005-02-18 Reply Admin

:
Whitespace is free if your time has no value ...

try{datReadr=dataCmd.ExecuteReader();}catch(Exception e){if(e.GetType().Equals(typeof(DataNotFoundException))){try{throw e;}catch(Exception e2){log.error(e2);throw new ManagedException(e.GetType().Name);}}else{throw new DataNotFoundException("Exception while importing operational data.");}}

This takes lesser time to understand [:P]

Playing Catch

Leave a comment on “Playing Catch”