1

I wrote this method and I want to handle UnexpectedFormatExceptions: more specifically, a MissingFieldException, an EmptyFieldException, an UnknownCardTypeException, an UnknownSpellCardException.

The problem is that I don't fully understand the idea of handling exceptions. Now, I made a class UnexpectedFormatExceptions (as mentioned earlier) and subclasses with the constructors and alike. Should I just add a try block to take the whole code and catch blocks for each exception? What would be the right course of action here?

public ArrayList<Card> loadCardsFromFile(String path) throws IOException, FileNotFoundException, UnexpectedFormatException { String currentLine = ""; FileReader fileReader = new FileReader(path); @SuppressWarnings("resource") BufferedReader br = new BufferedReader(fileReader); String[] currentsplit; ArrayList<Card> temp = new ArrayList<Card>(); while ((currentLine = br.readLine()) != null) { currentsplit = currentLine.split(","); if (currentsplit[0].equals("Monster")) { MonsterCard x = new MonsterCard(currentsplit[1], currentsplit[2], Integer.parseInt(currentsplit[5]),Integer.parseInt(currentsplit[3]), Integer.parseInt(currentsplit[4])); temp.add(x); } else { if (currentsplit[1].equals("Card Destruction")) { CardDestruction x = new CardDestruction(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Change Of Heart")) { ChangeOfHeart x = new ChangeOfHeart(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Dark Hole")) { DarkHole x = new DarkHole(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Graceful Dice")) { GracefulDice x = new GracefulDice(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Harpie's Feather Duster")) { HarpieFeatherDuster x = new HarpieFeatherDuster(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Heavy Storm")) { HeavyStorm x = new HeavyStorm(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Mage Power")) { MagePower x = new MagePower(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Monster Reborn")) { MonsterReborn x = new MonsterReborn(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Pot of Greed")) { PotOfGreed x = new PotOfGreed(currentsplit[1], currentsplit[2]); temp.add(x); } if (currentsplit[1].equals("Raigeki")) { Raigeki x = new Raigeki(currentsplit[1], currentsplit[2]); temp.add(x); } } } return temp; } 
9
  • Handle the exceptions at the points where it makes logical sense to. You should wrap the code in a try-catch where if the exception occurs, it will make little or no sense to continue processing the remaining code... Commented Apr 14, 2015 at 0:39
  • 2
    your code is doing multiple things which creates such complexity. if you break them into small functions, the exception will be clear Commented Apr 14, 2015 at 0:39
  • Why catch each exception by itself? Just do a try {} catch (Exception e) This is also a horrible way to try and turn yu-gi-oh into java - it'd be much better to write a general card glass, with enums as types for spell, trap, monster, and then give the cards properties such as names and descriptions. Having a separate class for each card isn't the proper way to do it, especially when yu-gi-oh has so many different cards... Commented Apr 14, 2015 at 0:42
  • 1
    @Aify, exceptions are designed for specific errors, you always want to capture most specific exceptions and process it first. If you just do try {} catch (Exception e), you ignored all exceptions design. Commented Apr 14, 2015 at 0:48
  • @billz can I add try-catch at each part that can throw the exception? Commented Apr 14, 2015 at 0:49

3 Answers 3

1

But I don't fully understand the idea of handling exceptions.

An analogy:

Normal code flow:

  A train is running on the track.

Exception flow:

  Train went off the track (derailed)

Exception Handling:

  Bringing the train back on track.


Should I just add a try block to take the whole code and catch blocks for each exception? or what should I do?

Catch only appropriate exception for the last block of statements. Try to keep the try-catch block as minimal as possible and only surround necessary codeblock with appropriate try-catch block.

Optionally you can also use this new syntax (Java 7):

try { // statements } catch (MissingFieldException|EmptyFieldException|UnknownCardTypeException|UnknownSpellCardException ex) { logger.log(ex); throw ex; } 
Sign up to request clarification or add additional context in comments.

Comments

0

"But I don't fully understand the idea of handling exceptions."

An Exception is just that the normal program flow could not be followed.

So, if the normal flow could not be followed, using Exception handling allows you to take action for the "not normal" flow.

FileNotFoundException is kind of a simple one to try to illustrate:

So

try { ...statements to open a file and read its contents... } catch (FileNotFoundException fnfe) { // maybe print out an error message to the user // or set a flag // etc } 

If you do not want a method to handle an Exeption, then you delegate the Exception handling to an 'outer' method or caller, by having the method 'throw the Exception' as you have in your code, but somewhere you'll need to do the try/catch to handle it, like I showed above.

Comments

0

Exceptions are quite a tricky topic that -at least IMO- is quite hard to master. Quite frequently there will be disagreements about what exceptions to throw, where to handle them or whether to throw them at all (and so on). If you want to master them, you will probably need tons of reading and especially of burning yourself with bad exception approaches. I'm going to be fairly generic at times, leaving a lot of the design decisions to you. However I made a few comments and suggestions specifically about your code.

Should I just add a try block to take the whole code and catch blocks for each exception?

No. No, no, no, NO! It may make your life hard when you try to find out which part of your code actually threw the Exception. But not only that. Search for "try catch everything" or something like that on Google / here to see other arguments - my answer is already long enough.

Personally, I'd split your method into three: I'd have a method that reads from a file and puts the contents into an ArrayList<String>, then I'd pass the ArrayList<String> to this method, the for each String in the ArrayList I would try to create a Card*. Separating them would also make your methods much easier to (unit) test. Here's the structure

public ArrayList<String> loadRowsFromFile(String path) throws IOException, FileNotFoundException { // whatever } // up to you if you want this to throw UnexpectedFormatException public ArrayList<Card> loadCardsFromFile(ArrayList<String> cardsList) throws UnexpectedFormatException { // whatever } public Card buildCardFromString(String card) throws MissingFieldException, EmptyFieldException, UnknownCardTypeException, UnknownSpellCardException { // whatever } 

I think it helps if you think about your method contract: what does my method need as input and what does my method produce based on that input? Should it work with any input? Should it work only with some inputs and throw exceptions and let the calling code handle them? If card or cardsList is null (or empty) you may want to throw an IllegalArgumentException or provide some default behaviour, depending on your needs - you need to think about them. Does it makes sense to produce a Card out of a null / empty String? Does it make sense to have an ArrayList<Card> out of a null / empty cardsList? If the answer is "No", by all means throw a (relevant) exception. Don't invent workarounds for bad input. It'll make your code a nightmare.

The easiest to analyze like this is loadRowsFromFile. This is a method that is expected to produce a list of rows out of a file. But if the path is invalid, it doesn't make sense to carry on, because it can't fulfill its contract: what ArrayList<Card> would be most sensible to produce out of an invalid file / path? In general, probably none. Thus, it makes sense to throw a relevant exception. The code that calls it needs to handle this exception or, if it's not expected or if it doesn't quite make sense to deal with this exception, it should throw it further up the call stack.

Having specific exceptions (like MissingFieldException, EmptyFieldException etc.) may clear up your code and help with debugging and exception resolution, but again, it really depends on where or how you want to handle them. For example, one layer may need specific exceptions and then it will rethrow them higher up where there's a catch for more general exceptions.

*You might actually want to look into the factory method pattern.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.