Comment On Code Reuse

He also seems to know that long variable names are way to expensive in size and speed.

Hahah, this one actually made me 'WTF!' out loud :D
Nice touch to this WTF is that it will probably actually be a few cycles slower than just using two try catch blocks (because of i and some extra conditional jumps)

As I read this, I thought on the inside "Wrong! No! This is wrong! :("

This is just going to be slower, and altogether pointless.

I assume that any half-decent compiler would unroll that loop to be just as efficient as it is without the loop.

Really. Which compiler do you use for your javascript?

I've come across the for-case loop several times; I think one of my coworkers actually thinks that's a legitimate development pattern.

"it will probably actually be a few cycles slower than just using two try catch blocks"

Why would you need two try catch blocks?

Javascript doesn't have try-catch.

Tweedle-Dink:

Javascript doesn't have try-catch.

Yes it does.

http://www.w3schools.com/js/js_try_catch.asp

It does not, however, have anything called IOException, so this is still Java after all.

Tim:

"it will probably actually be a few cycles slower than just using two try catch blocks"

Why would you need two try catch blocks?

Because otherwise, if the first operation fails (throws an exception) than the second one will also be skipped.

JavaScript has try-catch, and typically JavaScript is JIT-compiled. Anyway, what about JAVA?

?????????

I'm sure that there has to be some reasonable, rational, sane explanation for this but I can't quite think of one at the moment.

Still, they missed a trick; instead of a for-loop they could have a recursive function passing in a count parameter.

Tim:

"it will probably actually be a few cycles slower than just using two try catch blocks"

Why would you need two try catch blocks?

I'm assuming he's trying to close two streams.
baos and dos

The first attempt code snippet would be:

baos.close();
dos.close();
dos = null;
baos = null;

Now we decide that we need to handle exceptions:

try
{
baos.close();
dos.close()
}
catch (IOException ioe)
{ //Squelch, as this means the stream is probably gone already - Why is this a checked exception? I can't possibly do anything.
}
finally
{
baos =null;
dos = null;
}

Now, what happens if baos.close() throws an exception?
dos.close is never called.

Without getting into a whole whack of convoluted code involving multiple cases and checks, Two try blocks seems to be the best way to ensure that both streams are closed (if not already) and nulled.

Besides, it costs next to nothing to Try, it only costs when you actually throw an exception.

By the looks of things this is a true WTF (going to the original meaning). If my guess is right the variable baos is a ByteArrayOutputStream & dos is a DataOutputStream. The great thing about these two classes is that they both inherit from OutputStream so all that's required is a tiny method like:

public static close(OutputStream os) {

  try {

    os.close();

  }

  catch(IOException e) {

    //handle & log exception

  }

}

It's like someone from Sun thought that a sensible programmer might like to program a method like this using the superclass. I'm sure there's a name for when you program like this but I just can't remember :)

Try:

org.apache.commons.io.IOUtils.closeQuietly(baos);
org.apache.commons.io.IOUtils.closeQuietly(dos);

I went a step further than Apache and made a closer that will call close() on anything that has one. If and when somebody needs to handle one of those exceptions, I'll add an exception handler argument.

AnonY:

By the looks of things this is a true WTF (going to the original meaning). If my guess is right the variable baos is a ByteArrayOutputStream & dos is a DataOutputStream. The great thing about these two classes is that they both inherit from OutputStream so all that's required is a tiny method like:

public static close(OutputStream os) {

  try {

    os.close();

  }

  catch(IOException e) {

    //handle & log exception

  }

}

It's like someone from Sun thought that a sensible programmer might like to program a method like this using the superclass. I'm sure there's a name for when you program like this but I just can't remember :)

It goes to the large WTF that is checked exceptions.
I'm not against them per se, but I find the core library forces me to catch exceptions that I cannot possibly do anything about.

I want to close a stream. I can't, because the stream is already closed. This is on par with that classic windows WTF, "Cannot delete foo as there is not enough disk space. Try deleting some files"

I've lost the stream, it's up to GC to clean up, all I can do is null the reference, which I was going to do anyways.

AnonY:

It's like someone from Sun thought that a sensible programmer might like to program a method like this using the superclass. I'm sure there's a name for when you program like this but I just can't remember :)

What, intelligence?

I'm guessing that code would look something like this...(forgive any syntax errors)

for( OutputStream os : { baos, dos } )

{

	try

	{

		os.close();

	}

	catch( Exception e ) {}

	

	os = null;

	

}

hope no one heard me exclaim "Now that is funny" instead of laughing.

hmm...i suspect baos is a ByteArrayOutputStream in that case close() does nothing, and dos is a DataOutputStream and in it's close() it closes the os (the only operation that can throw IOEx) on which was built (baos I think).
so...a single try{dos.close()}catch(... is more than enough.

Premature optimization is a distressing affliction, but it's happened to lots of guys.

Stan:

I went a step further than Apache and made a closer that will call close() on anything that has one. If and when somebody needs to handle one of those exceptions, I'll add an exception handler argument.

Don't forget
a) adding "finally" handler
b) adding customizable behaviour, so that one can specify a method other than "close", or maybe even arbitrary block of code.

Something like
public delegate void VoidMethod();

public static void CloseQuietly(VoidMethod closingMethod, Action<Exception> exceptionHandler, VoidMethod finallyHandler);

to be called like

CloseQuietly(
new delegate() { someStream.Close(); },
new delegate(Exception exc) { Console.WriteLine("Error closing stream: " + exc); },
new delegegate() { someStream = null; });


That would be invaluable addition to this site!

Vector:

AnonY:

It's like someone from Sun thought that a sensible programmer might like to program a method like this using the superclass. I'm sure there's a name for when you program like this but I just can't remember :)

What, intelligence?

I'm guessing that code would look something like this...(forgive any syntax errors)

for( OutputStream os : { baos, dos } )

{

	try

	{

		os.close();

	}

	catch( Exception e ) {}

	

	os = null;

	

}

I hope you're not advocating the use of such a loop... ;o)

BTW, nullifying the reference here is pointless - the variable declared in the head of a for/in loop has the loop body as its scope, i.e. it is redeclared on the stack in each iteration anyway (which can be told by the fact that you may declare it final - not something you normally want to do with a loop variable ;o), always has exactly one value, i.e. the currently processed entry of the Iterable (usually an array or collection) and can not exist outside/after of the loop body.

The code surely qualifies as a wtf, but i think all the speculation about this being an attempted performance optimization is flat-out wrong. There are still two try-catch statements which get executed in this code, so, no difference from doing two consecutive try-catch statements. As an aside, it doesn't suffer the "if first close throws exception second doesn't execute" problem.

My guess is that the coder thought this was a clever way to make his/her code shorter and avoid having the extra lines writing this out twice would entail. I just don't see looking at the code that there's any attempt to mangle the code in the name of performance, just misguided readibility. Of course, better solution there would be to have a "close(OutputStream os){}" method which contained the try-catch and close logic, and invoke the method twice, consecutively, with different arguments.

Look at me! I'm on the internets!:

It goes to the large WTF that is checked exceptions.
I'm not against them per se, but I find the core library forces me to catch exceptions that I cannot possibly do anything about.

I want to close a stream. I can't, because the stream is already closed. This is on par with that classic windows WTF, "Cannot delete foo as there is not enough disk space. Try deleting some files"

I've lost the stream, it's up to GC to clean up, all I can do is null the reference, which I was going to do anyways.

It is in clean-up code like this I really wish I had VB's

On Error Resume Next

...And only in code like this...!

A quality chunkette of code.

I especially like the way he uses a switch during an attempt to optimise code!

You're supposed to close streams in finally blocks

Look at me! I'm on the internets!:

Tim:

"it will probably actually be a few cycles slower than just using two try catch blocks"

Why would you need two try catch blocks?

I'm assuming he's trying to close two streams.
baos and dos

The first attempt code snippet would be:

baos.close();
dos.close();
dos = null;
baos = null;

Now we decide that we need to handle exceptions:

try
{
baos.close();
dos.close()
}
catch (IOException ioe)
{ //Squelch, as this means the stream is probably gone already - Why is this a checked exception? I can't possibly do anything.
}
finally
{
baos =null;
dos = null;
}

Now, what happens if baos.close() throws an exception?
dos.close is never called.

Without getting into a whole whack of convoluted code involving multiple cases and checks, Two try blocks seems to be the best way to ensure that both streams are closed (if not already) and nulled.

Besides, it costs next to nothing to Try, it only costs when you actually throw an exception.

It's clearly the Visual Basic “on error resume next” pattern. The low-level implementation of that VB.NET feature looks basically the same way.

HEY. I've done something similar.

In college, I had an ASM class and one of our projects was to implement an array handling program. My solution to how to manipulate the array was something like:

do_stuff_here:

; Do Stuff here. mov ax, bx etc

jmp dx

later in the code

; set up for do_stuff_here. Set array[x][y] as putting offset x and offset y into something like ax, and bx, and the result into si. Don't ask why I chose SI. I don't know

lea dx, return_to_address
jmp do_stuff_here


return_to_address:

now, I could've written a macro or a proper procedure, but I didn't like how masm handled macros and I only needed it within a single procedure. The syntax was scary and not portable. I was also in college, so portability wasn't a real problem, but hey, I'm an American, we do things with out thinking them through.

That's why C#'s using blocks rock.

try
{
using (Stream baos = new Stream())
{
using (Stream dos = new Stream())
{
// ....
}
}
} catch (IOException e) { // .... }

If an IOException occurs inside the inner code block, or even if either of the streams dispose (and hence close) methods throws an error the other stream still gets closed.

I <3 using blocks

boolean atFirstYouDontSucceed = true;



if (atFirstYouDontSucceed) {

   try {

       try {

            again();

       } catch (SomeWtfException wtf1) {

         throw new SomeOtherWtfException2(wtf2);

       } finally {

         throw new SomeOtherWtfException2();

       }

   } catch (SomeOtherWtfException wtf2) {

     throw FileNotFoundException(); // operation succeeded!

   } finally {

     // cry!

   }

}

yafake:

typically JavaScript is JIT-compiled.

<p>Er... definitely not. To my knowledge, no browser has a JITed JS engine yet, the first one will probably be Firefox when they'll have included the TAMARIN JS VM in the trunk. The current Gecko JS VM is not JITed, MSIE's JScript engine is not JITed, Opera's Javascript engine is not JITed and Safari's JS engine is not JITed either.</p>

Don't forget to throw an OperationSuccessful exception when you're finished.

ShelteredCoder:

Try:

org.apache.commons.io.IOUtils.closeQuietly(baos);
org.apache.commons.io.IOUtils.closeQuietly(dos);

Please don't try that. Don't try it at all.

Repeat after me: dependencies make code fragile.

One more time: dependencies make code fragile.

Don't you ever get pissed off when you try to compile some open-source code, only to learn you need to go out and download fifty of the author's favorite pet utility libraries?

Programmers need to start treating external dependencies like the cost they actually represent. Whether it's C or Java, each dependency makes the program dramatically less portable, because either the user needs to get ahold of all those libraries in order to run, or, in the case of C, the final executable is statically linked and therefore significantly bigger.

And for what? In the case of this (and most other) Apache Commons "utility," it's to save a few lines. There is no way in this world that it is a better design to depend on an external library and put users through all that hassle, than it is to just put in four lines doing the close yourself. Yes, even if it means, heaven forbid, writing a few lines to do a secondary try-catch.

Think of each possibility as having a weight: writing out your try-catch has a very light weight, while incurring a dependency on a third-party library bears a heavy weight. Weigh each option before adding a depedency to your code. Please.

The article is *exactly* why the Closeable interface was added in Java 5.

http://java.sun.com/j2se/1.5.0/docs/api/java/io/Closeable.html

http://java.sun.com/j2se/1.5.0/docs/api/java/io/Closeable.html idiom:

private void safeClose(Closeable c)
{
try
{
if (c != null)
c.close();
}
catch(IOException e)
{
// Ignore
}
}

I don't know much about Java, but looking at that makes me want to run screaming to somewhere safe and cosy where I can hide from the world. If the WTF isn't in the code, it sure is in the language!

This is obviously in a language where loops are automatically parallelized by default, like Fortress.
Silly goose!

Hehe... that one actually did make me say WTF. Good one :)

VGR:

ShelteredCoder:

Try:

org.apache.commons.io.IOUtils.closeQuietly(baos);
org.apache.commons.io.IOUtils.closeQuietly(dos);

Please don't try that. Don't try it at all.

Repeat after me: dependencies make code fragile.

One more time: dependencies make code fragile.

Don't you ever get pissed off when you try to compile some open-source code, only to learn you need to go out and download fifty of the author's favorite pet utility libraries?

Programmers need to start treating external dependencies like the cost they actually represent. Whether it's C or Java, each dependency makes the program dramatically less portable, because either the user needs to get ahold of all those libraries in order to run, or, in the case of C, the final executable is statically linked and therefore significantly bigger.

And for what? In the case of this (and most other) Apache Commons "utility," it's to save a few lines. There is no way in this world that it is a better design to depend on an external library and put users through all that hassle, than it is to just put in four lines doing the close yourself. Yes, even if it means, heaven forbid, writing a few lines to do a secondary try-catch.

Think of each possibility as having a weight: writing out your try-catch has a very light weight, while incurring a dependency on a third-party library bears a heavy weight. Weigh each option before adding a depedency to your code. Please.

Yeah but that would require people to actually write some of their own code, from their own heads... and if this blog proves anything, it's that people don't know how to write their own code and when they do, it wants to make you say "WTF?!"

Good developers know that, (1) Exception Handling is costly and might use thousands of CPU cycles to process which, in turn could take several nanoseconds

Ahem. Good coders know that "thousands of CPU cycles" and "several nanoseconds" are only the same if your processor is running at 1 THz. The current state of the art is more like 1 GHz, and being off by three orders of magnitude on a performance calculation is not cool.

This time your errors cancel each other out. Exceptions shouldn't take thousands of CPU cycles. Even if they did, if I correctly read your sarcasm, you already know that they probably shouldn't be thrown often enough for several microseconds to be a big deal.

WTF! How did that pass code review without the unexpected default: case throwing an appropriate exception?

slamb:

Exceptions shouldn't take thousands of CPU cycles.

In some architectures they can...

you already know that they probably shouldn't be thrown often enough for several microseconds to be a big deal

... and in some applications they do. ;)

despair:

slamb:

Exceptions shouldn't take thousands of CPU cycles.

In some architectures they can...

you already know that they probably shouldn't be thrown often enough for several microseconds to be a big deal

... and in some applications they do. ;)

Well this code will still go through a try catch block twice, the only optimization this could possibly effect would be in the compilation (though that assumes processing the try catch is slower than processing the for loop, which I sort of doubt).

BTW, I again remind you, if you think this is a WTF, at least they weren't catching a Throwable and then doing nothing about it. I just corrected yet another one of those today...

Equivalent C++ code:

Old Wolf:

Equivalent C++ code:

You win 1000 Internets, my friend.

jock:

Besides, it costs next to nothing to Try, it only costs when you actually throw an exception.

True for Java. dotNet (1.1 at least) pays a performance price for every entry and exit from guarded code, but performs faster during exception handling.

I prefer the Java model. Exceptions are supposed to be exceptional after all.

My favorite Exception WTF is when I caught a coworker using different exceptions to implement GOTOs in Java. WTF!

> I've come across the for-case loop several times; I think one of my coworkers actually thinks that's a legitimate development pattern.

Crikey, I also once came across a for-loop with a switch/case inside for each iteration, it felt like I had just witnessed a pregnant unicorn or something equally mystical. Ah well...

Robert Briggs:

?????????

I'm sure that there has to be some reasonable, rational, sane explanation for this but I can't quite think of one at the moment.

<SNIP>

I'll try: Originally the catch-block was waaaaaaay more complex for whatever reasons (corporate standards or similar maybe - I'm just guessing). So the developer went ahead and used the "for-case-(ummm)-pattern" to avoid code repeatition. I also surmise that this code fragment shown is about the only place in the emcompassing module where files are closed and so this is the only place where a try-catch-block like this is actually needed. Later during the cause of further development the catch-block was simplified to what is shown. When they did that they should eliminated the "for-case-pattern" and should have serialized into two separate try-catch-statements ... which they didn't do.

slamb:

Good developers know that, (1) Exception Handling is costly and might use thousands of CPU cycles to process which, in turn could take several nanoseconds

Ahem. Good coders know that "thousands of CPU cycles" and "several nanoseconds" are only the same if your processor is running at 1 THz. The current state of the art is more like 1 GHz, and being off by three orders of magnitude on a performance calculation is not cool.

This time your errors cancel each other out. Exceptions shouldn't take thousands of CPU cycles. Even if they did, if I correctly read your sarcasm, you already know that they probably shouldn't be thrown often enough for several microseconds to be a big deal.

Alert - Alert - Severe sarcasm detector failure .....

And (ahem): first on page 2

This reminds me of code in our project that was written before i joined. In this code you will see things like

for (i = 0; i < 5; i++)

{

  if (i == 0)

     dothing();

  else if (i == 1)

     dootherthing();

  else

     dothisthing();

}