[streek405]

When I try closing my file I get an error stating that my close statement needs to be in [another] try catch statement, why?

Here is what I have:

public void start(){//reads the input files and stores into hashmap
		
		System.out.println("START!!!");
		startCalled = true;		
		try {
			fr = new FileReader(getInfileName());
			br = new BufferedReader(fr);
			Insert(br);
			
			br.close();//error stating I need a separate exception
		} catch (FileNotFoundException e) {
			e.printStackTrace();
		}		
	}

*Update before even posting* Is it because I am trying to catch something specific in line 11? If I made my catch into

catch (Exception e)

instead, would that eliminate the error?

anndddd that fixed my error

[Koekje]

Yes, because the error which can be thrown is not FileNotFoundException nor a subclass of it. The close method throws an other kind of exception and you should catch and handle it accordingly.

Catching all exceptions with catch(Exception) is generally a bad idea (you can find loads of information as to why).

You can caatch multiple exceptions for example by

catch (SomeException | SomeOtherException) {...} 

[streek405]

Koekje, on 12 November 2014 - 05:22 PM, said:

Yes, because the error which can be thrown is not FileNotFoundException nor a subclass of it. The close method throws an other kind of exception and you should catch and handle it accordingly.

Catching all exceptions with catch(Exception) is generally a bad idea (you can find loads of information as to why).

You can caatch multiple exceptions for example by

catch (SomeException | SomeOtherException) {...} 

Ok thanks but what if I dont know the type of error that I could be getting? What type of exception should I through for the close method? would I do something like:

catch(Exception e | "CloseException" e)

?

[jon.kiparsky]

The great thing about Java is that you can always find out what you need to know. Checked exceptions are always documented - see the javadoc for the class you're using. Or, if you're using some sort of rebel code that doesn't have documentation, the compiler will tell you what you need to be catching.

Quote

Catching all exceptions with catch(Exception) is generally a bad idea (you can find loads of information as to why).

This is very good advice. The "why" of it is pretty obvious: when you catch an exception, you know that something might be going wrong, and you know how it could be going wrong. These are real-world failure cases that you can deal with ahead of time. Sometimes you'll just say "can't cope" and report and error to the user, but most of the time you'll be able to recover, since you know exactly what happened.
Unless you don't.

Right: unless, you've compressed all of the information about what went wrong into one blob of "Exception e" - now you know nothing again. Oops.

Quote

You can caatch multiple exceptions for example by

catch (SomeException | SomeOtherException) {...} 

It's hard to see how this is distinguishable from catching Exception e in its practical effects. It puts you in exactly the same position as if you used the simpler form.


I would further add that in general you should make your try blocks as short as possible. They should wrap just the atomic operation that involves the failure - just the things that you will wish hadn't happened if an Exception arises. They should not wrap multiple independent failure cases. The reason for this is also simple: you want to make your life as simple as possible. If opening the file fails, you probably want to handle that (oops, that file is befuckled, can't deal). If reading from the file fails (oops, someone yanked the USB stick after you opened the file) you want to handle that separately. If closing the file fails (oops, maybe there wasn't room on the file system) then you want to deal with that separately.

[g00se]

If you're using >= Java 7, you can make your life easier by using 'try with resources'

[cfoley]

Oooh people get all hot and bothered about exceptions. Here is my take:

catch (FileNotFoundException e) {
            e.printStackTrace();
        }     

You probably don't mean this. This says "write an error message and then continue". Do you really want to continue? I don't think so. I think that you don't know what to do if the file is not there, at least in the context of this method. I bet you also don't know what to do if there is an IO error. That's OK. Neither do I. The correct thing to do is to say you don't know what to do:

//reads the input files and stores into hashmap
public void start() throws FileNotFoundException, IOException {
	System.out.println("START!!!");
	startCalled = true;		
	fr = new FileReader(getInfileName());
	br = new BufferedReader(fr);
	Insert(br);	
	br.close();
}

This looks a lot better. It's more readable, it helps prevent things getting into a dangerous state and it lets the caller know things might fail. If the caller can do something sensible in the event of failure then it can catch an exception (of the appropriate type), otherwise it can also throw the exceptions. Eventually, someone in the chain of callers will know what to do or the exception will bubble all the way out, stop the thread and write the stack trace to err. This is probably the behaviour you really want anyway.

There is still a problem with my last snippet. It might leave the file in an open state. In ancient versions of Java you had to do all sorts of nonsense wrapping layers of try catches. As goose pointed out, you can use try with resources to make sure it gets automatically closed for you:

//reads the input files and stores into hashmap
public void start() throws FileNotFoundException, IOException{
	System.out.println("START!!!");
	startCalled = true;	
	try (br = new BufferedReader(new FileReader(getInfileName()));)/>	{
		Insert(br);	
	}	
}

That comment is lying to you. The method is doing the reading stuff but it's also doing some logging stuff. Let's sort out a method for each responsibility. This looks better to me. YMMV.

public void start() throws FileNotFoundException, IOException{
	logStarting();
	readIntoHashmap();
}

private void logStarting() {
	System.out.println("START!!!");
	startCalled = true;	
}

private void readIntoHashmap() {
	try (br = new BufferedReader(new FileReader(getInfileName()));)/>	{
		Insert(br);	
	}	
}

[g00se]

Yes, that's all good advice. The only thing i would say is don't roll your own logging, if only for the simplest reason that you can't turn it off without changing and recompiling your code. Use a logging framework - java.util.logging if you must, though imo that's broken for declarative control

[cfoley]

True that.

You know, I didn't realise it was logging until I was thinking of a name for the extracted method. Now I think about it, maybe it's not logging at all. Maybe that boolean has some other function and the println is an artifact of some ongoing debugging.

How about this, (although it could still be improved by following g00se's advide)

public void start() throws FileNotFoundException, IOException{
	startCalled = true;	
	debug("START!!!");
	readIntoHashmap();
}

private void debug(String s) {
	if (showDebug)
		System.out.println(s);
}

private void readIntoHashmap() {
	try (br = new BufferedReader(new FileReader(getInfileName()));)/>	{
		Insert(br);	
	}	
}