views:

106

answers:

3

Hi, I'm having a little bit of trouble implementing the following method while handling the 3 exceptions I'm supposed to take care of. Should I include the try/catch blocks like I'm doing or is that to be left for the application instead of the class design?

Uml diagram of the program: http://yfrog.com/9htuoj

The method says I'm supposed to implement this:

public Catalog loadCatalog(String filename)
         throws FileNotFoundException, IOException, DataFormatException

This method loads the info from the archive specified in a catalog of products and returns the catalog.

It starts by opening the file for reading. Then it proceeds to read and process each line of the file.

The method String.startsWith is used to determine the type of line:

  • If the tipe of line is "Product" , the method readProduct is called.
  • If the tipe of line is "Coffee" , the method readCoffee is called.
  • If the tipe of line is "Brewer" , the method readCoffeeBrewer is called.

After the line is processed, loadCatalog adds the product (product, coffee or brewer) to the catalog of products.

When all the lines of the file have been proccesed, loadCatalog returns the Catalog of products to the method that makes the call.

This method can throw the following exceptions:

  • FileNotFoundException — if the files specified does not exist.
  • IOException — If there is an error reading the info of the specified file.
  • DataFormatException — if a line has errors(the exception must include the line that has the wrong data)

Here is what I have so far:

public Catalog loadCatalog(String filename) throws FileNotFoundException, IOException, DataFormatException{

    String line = "";


    try {
        BufferedReader stdIn = new BufferedReader(new FileReader("catalog.dat"));

            try {
                BufferedReader input = new BufferedReader(new FileReader(stdIn.readLine()));

                while(! stdIn.ready()){
                    line = input.readLine();


                    if(line.startsWith("Product")){

                        try {
                            readProduct(line);
                        } catch(DataFormatException d){
                            d.getMessage();
                        }


                    } else if(line.startsWith("Coffee")){

                        try {
                            readCoffee(line);

                        } catch(DataFormatException d){
                            d.getMessage();
                        }


                    }  else if(line.startsWith("Brewer")){

                        try {
                            readCoffeeBrewer(line);
                        } catch(DataFormatException d){
                            d.getMessage();
                        }

                    }
                }


            } catch (IOException io){
                io.getMessage();

            }


    }catch (FileNotFoundException f) {

        System.out.println(f.getMessage());
    }


    return null;
}
+4  A: 

It depends on whether you want the class or another portion of the application that is using it to handle the exception and do whatever is required.

Since the code that will use the loadCatalog() probably won't know what to do with a file I/O or format exception, personally, I'd go with creating an exception like CatalogLoadException and throw it from within the loadCatalog() method, and put the cause exception (FileNotFoundException, IOException, DataFormatException) inside it while including an informative message depending on which exception was triggered.

try {
         ...
    //do this for exceptions you are interested in.
    } catch(Exception e) {
         //maybe do some clean-up here.
         throw new CatalogLoadException(e); // e is the cause.
    }

This way your loadCatalog() method will only throw one single and meaningful exception.

Now the code that will use loadCatalog() will only have to deal with one exception: CatalogLoadException.

loadCatalog(String filename) throws CatalogLoadException

This also allows your method to hide its implementation details so you do not have to change its "exception throwing" signature when the underlying low level structure changes. Note that if you change this signature, every piece of code would need to change accordingly to deal with the new types of exceptions you have introduced.

See also this question on Exception Translation.


Update on the throwing signature requirement:

If you have to keep that signature then you don't really have a choice but to throw them to the application and not catch them inside the loadCatalog() method, otherwise the throws signature would be sort of useless, since we aren't going to throw the exact same exception that we have just dealt with.

Bakkal
@Bakkal thanks for the great piece of advice, it does sound like a better design technique to throw only 1 exception that handles all of them. Ill give it a shot from now on. However, I forgot to mention that for this particular exercise(for school) I have to implement the method exactly as described because afterwards I have to make it run through a premade Tesfile app and make sure it compiles.
edu222
gotcha, thanks for the help :)
edu222
+1  A: 

The general idea is that you percolate exceptions up to the appropriate place to handle them. I am going to guess that your instructor expects them to be handled in main. In this case I can guess that because of the throws clause you were given. A simple rule of thumb is that if the method declares the exception in the throws clause you do not catch it in that method. So the method you are writing should have no catch statements.

To do that you would change your code something like:

public Catalog loadCatalog(String filename) 
    throws FileNotFoundException, 
           IOException, 
           DataFormatException
{
    String line = "";

    BufferedReader stdIn = new BufferedReader(new FileReader("catalog.dat"));
    BufferedReader input = new BufferedReader(new FileReader(stdIn.readLine()));

    while(!stdIn.ready())
    {
        line = input.readLine();

        if(line.startsWith("Product"))
        {
            readProduct(line);
        } 
        else if(line.startsWith("Coffee"))
        {
            readCoffee(line);
        }  
        else if(line.startsWith("Brewer"))
        {
            readCoffeeBrewer(line);
        }
    }

    return null;
}

and then in the method (presumably main) that calls loadCatalog you would have:

try
{
   loadCatalog(...);
}
catch(FileNotFoundException ex)
{
    ex.printStackTrace(); 
}
catch(IOException ex)
{
    ex.printStackTrace(); 
}
catch(DataFormatException ex)
{
    ex.printStackTrace(); 
}

replacing the printStackTrace with something appropriate.

That way the method, loadCatalog, doesn't deal with displaying the error messages, so you can call the method in GUI or console code and the code that calls it can choose how to display the error to the user (or deal with it in some way).

TofuBeer
good stuff, thanks
edu222
A: 

Here is an excellent article of Heinz Kabutz, dealing with exception handling.

http://www.javaspecialists.eu/archive/Issue162.html

Sylvain M
thanks for passing it over, as soon as I get a chance Ill read it :)
edu222