Executing an operator in an RPN evaluator

Question

Is there any way to take this chunk of an else if and make it its own method? I would need the values of valid and throwLine() to be done in the method containing this if else statement, but I'm around 45 lines in my method which is 15 more than I'm allowed to have for simplicity.

 else if(isOperator(readIn))
         {
            System.out.print(readIn);
            if(myStack.isEmpty())
               valid = false;
            else
               op2 = (Fraction)myStack.pop();

            if(myStack.isEmpty())
            {
               throwLine(readIn);
               valid = false;
            }
            else
            {
               runTheOperator(op2, op1, readIn);
               readIn = stdin.next();
            }

Answer 1

As @DavidHarkness points out, you didn't provide enough context for this question to make sense. However, I see that this code comes from a doTypeCheck() method, which is at the heart of your RPN calculator implementation. I'll reproduce that method here, along with its call site:

private void doTypeCheck(Fraction op1, Fraction op2)
{
    Fraction answer = null;
    String readIn = "";
    boolean valid = true;
    readIn = stdin.next();

    while(!readIn.equals("#") && valid == true)
    {
       if(!isOperator(readIn) && isOperand(readIn))
       {
          processOperand(readIn);
          readIn = stdin.next();
       }
       else if(isOperator(readIn))
       {
          System.out.print(readIn);
          if(myStack.isEmpty())
             valid = false;
          else
             op2 = (Fraction)myStack.pop();

          if(myStack.isEmpty())
          {
             valid = false;
             throwLine(readIn);
          }
          else
          {
             runTheOperator(op2, op1, readIn);
             readIn = stdin.next();
          }
       }
       else
       {  
          System.out.print(readIn);
          valid = false;
          throwLine(readIn);
       }
    }
    System.out.println();
    if(myStack.isEmpty())
       valid = false;
    else
       answer = (Fraction)myStack.pop(); 
    if(!myStack.isEmpty())
       valid = false;
    checkMessageValid(valid, answer);
}

private void runOnce(int count)
{
    Fraction op1 = null;
    Fraction op2 = null;
    clear(myStack, myQueue);

    System.out.print("Expression " + count++ + " is: ");
    doTypeCheck(op1, op2); 
 }

---

The root problem is that your method has no clear purpose, and tries to do many things:

• Read tokens from the input stream, and decide whether they are operands or operators
• If it sees invalid input, discard the rest of the input and display an error
• Pop an operand from the stack, with error handling
• Feed two operands to the calculating function
• Check that the stack is empty afterwards
• Display the answer

As I mentioned in my other answer, that's not a good way to write an RPN calculator.

The main loop of an RPN evaluator should probably look something like this:

try {
    while (in.hasNext()) {
        String token = in.next();
        RpnOperator op = RpnOperator.forSymbol(token);
        if (op != null) {
            op.operate(calcStack);
        } else {
            calcStack.push(new Fraction(token));
        }
    }
    System.out.println(calcStack.peek());
} catch (EmptyStackException emptyStack) {
    System.out.println("Error: empty stack");
}

That's all! To summarize:

• Let the operators manipulate the stack. The main loop just needs to dispatch to the appropriate operators.
• Use exceptions for error handling.

Answer 2

Well, if you are not wanting to change all your code as others suggested, I would use the following version:

private boolean tryFurtherOperator(String curLine, Stack stack, Fraction op1) {
    boolean valid = false;
    if(stack.isEmpty()){
        throwLine(curLine);
    } else {
        Fraction op2 = (Fraction)stack.pop();
        runTheOperator(op2, op1, curLine);
        valid = true;
    }
    return valid;
}


/* etc ...*/ else if(isOperator(readIn)){
    System.out.print(curLine);

    if(tryFurtherOperator(curLine, stack, op1)){
        curLine = stdin.next();
    }
}