Java try/catch method [closed]

Question

Closed. This question needs debugging details. It is not currently accepting answers.

Edit the question to include desired behavior, a specific problem or error, and the shortest code necessary to reproduce the problem. This will help others answer the question.

Closed 8 years ago.

Improve this 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
– SomeDude, Commented May 24, 2017 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
– EJoshuaS - Stand with Ukraine, Commented May 24, 2017 at 15:44

Itzik Shachar · Accepted Answer · 2017-05-24 15:51:16Z

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, 2017 at 15:51

answered May 24, 2017 at 15:44

Itzik Shachar

7545 silver badges16 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

EJoshuaS - Stand with Ukraine · Accepted Answer · 2017-05-24 15:50:50Z

0

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.

answered May 24, 2017 at 15:50

EJoshuaS - Stand with Ukraine

12.2k63 gold badges59 silver badges86 bronze badges

1 Comment

salivad Over a year ago

in doing what you suggested, there is an "Unhandled IOException" which then suggests adding a try/catch method around it

Collectives™ on Stack Overflow

Java try/catch method [closed]

2 Answers 2

Comments

1 Comment

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Linked

Related