Java try/catch method

Question

I'm new to Java and my function has a lot of try/catch blocks that I would like to clean up. I wanted to take each section and put it in a separate private helper method and only call a few functions within the main function, but when I do so, I get a java.util.NoSuchElementException for Scanner.

Here is the original function. Any help would be much appreciated.

 public void playGame(List<Card> deck, FreecellOperations<Card> model, int numCascades,
                   int numOpens, boolean shuffle) {
try {
  Scanner scan = new Scanner(rd);

try {
  Objects.requireNonNull(model);
  Objects.requireNonNull(deck);
} catch (NullPointerException npe) {
  throw new IllegalArgumentException("Cannot start game with null parameters.");
}

try {
  model.startGame(deck, numCascades, numOpens, shuffle);
 } catch (IllegalArgumentException iae) {
  ap.append("Could not start game. " + iae.getMessage());
  return;
 }

  ap.append(model.getGameState() + "\n");
  while (!model.isGameOver()) {
    String source = scan.next();
    if (source.substring(0, 1).equals("q") || source.substring(0, 1).equals("Q")) {
      ap.append("Game quit prematurely.");
      return;
    }
    String cardIndex = scan.next();
    if (cardIndex.substring(0, 1).equals("q") || cardIndex.substring(0, 1).equals("Q")) {
      ap.append("Game quit prematurely.");
      return;
    }
    String destination = scan.next();
    if (destination.substring(0, 1).equals("q") || destination.substring(0, 1).equals("Q")) {
      ap.append("Game quit prematurely.");
      return;
    }

    int pileNumber = 0;
    PileType sourceType = null;
    boolean isValidSource = false;
    while (!isValidSource) {
      try {
        switch (source.charAt(0)) {
          case 'F':
            sourceType = PileType.FOUNDATION;
            pileNumber = this.validMoveCheck(source, 4);
            isValidSource = true;
            break;
          case 'O':
            sourceType = PileType.OPEN;
            pileNumber = this.validMoveCheck(source, numOpens);
            isValidSource = true;
            break;
          case 'C':
            sourceType = PileType.CASCADE;
            pileNumber = this.validMoveCheck(source, numCascades);
            isValidSource = true;
            break;
          default:
            throw new IllegalArgumentException();
        }
      } catch (IllegalArgumentException iae) {
        ap.append("Invalid source pile. Try again.\n");
        source = scan.next();
        if (source.equals("q") || source.equals("Q")) {
          ap.append("Game quit prematurely.");
          return;
        }
      }
    }
    int cardNum = 0;
    boolean isValidCard = false;
    while (!isValidCard) {
      try {
        cardNum = Integer.parseInt(cardIndex);
        isValidCard = true;
      } catch (NumberFormatException nfe) {
        ap.append("Invalid card number. Try again.\n");
        cardIndex = scan.next();
        if (cardIndex.equals("Q") || cardIndex.equals("q")) {
          ap.append("Game quit prematurely.");
          return;
        }
      }
    }

    PileType destType = null;
    int destPileNum = 0;
    boolean isValidDest = false;
    while (!isValidDest) {
      try {
        switch (destination.charAt(0)) {
          case 'F':
            destType = PileType.FOUNDATION;
            destPileNum = this.validMoveCheck(destination, 4);
            isValidDest = true;
            break;
          case 'C':
            destType = PileType.CASCADE;
            destPileNum = this.validMoveCheck(destination, numCascades);
            isValidDest = true;
            break;
          case 'O':
            destType = PileType.OPEN;
            destPileNum = this.validMoveCheck(destination, 4);
            isValidDest = true;
            break;
          default:
            throw new IllegalArgumentException();
        }
      } catch (IllegalArgumentException iae) {
        ap.append("Invalid destination pile. Try again.\n");
        destination = scan.next();
        if (destination.equals("q") || destination.equals("Q")) {
          ap.append("Game quit prematurely.");
          return;
        }
      }
    }
    try {
      model.move(sourceType, (pileNumber - 1), (cardNum - 1), destType, (destPileNum - 1));
      ap.append(model.getGameState() + "\n");
    } catch (IllegalArgumentException iae) {
      ap.append("Invalid move. Try again. " + iae.getMessage() + "\n");
    }
  }
  ap.append("Game over.");
} catch (IOException ioe) {
  return;
}
}

Show your modified code and the stacktrace leading to `NoSuchElementException` — SomeDude, May 24 '17 at 15:44
I don't understand why you're using exception handling in these cases. For example, why throw an `IllegalArgumentException` in the `default` block? Why not just run the code in that `catch` block "directly"? — EJoshuaS - Stand with Ukraine, May 24 '17 at 15:44

Itzik Shachar · Accepted Answer · 2017-05-24T15:51:16.733

3

First, In order not to get java.util.NoSuchElementException, you need to check if the next line exists using hasNextLine(). Add that check in your while loop:

while (!model.isGameOver() && scan.hasNextLine()) {
...
}

Second, you got pretty good code styling tips in the other comments here, I suggest you to take them :)

edited May 24 '17 at 15:51

answered May 24 '17 at 15:44

Itzik Shachar

744
5
16

score 0 · Answer 2 · answered May 24 '17 at 15:50

A few comments:

First, you can replace a lot of these try/catch blocks with simple if statements (or eliminate them altogether).

For example:

default:
        throw new IllegalArgumentException();
    }
  } catch (IllegalArgumentException iae) {
    ap.append("Invalid destination pile. Try again.\n");
    destination = scan.next();
    if (destination.equals("q") || destination.equals("Q")) {
      ap.append("Game quit prematurely.");
      return;
    }
  }

Why not just do:

default:
    ap.append("Invalid destination pile. Try again.\n");
    destination = scan.next();
    if (destination.equals("q") || destination.equals("Q")) {
      ap.append("Game quit prematurely.");
      return;
    }
   break;

or something like that instead? Why bother with the exception?

Also, this logic is incorrect:

cardNum = Integer.parseInt(cardIndex);
isValidCard = true;

The fact that it's an integer doesn't prove that it's a valid card. What if someone entered 5,321? Clearly, that is an int, but it's not an actual card. Also, see here (as well as its duplicates) for ways to encapsulate this.

in doing what you suggested, there is an "Unhandled IOException" which then suggests adding a try/catch method around it — salivad, May 24 '17 at 17:49

Java try/catch method

2 Answers2