Why isn't my program returning the value of my expression?

Question

I am currently taking an AP Computer Science class in my school and I ran into a little trouble with one of my projects! The project requires me to create a calculator that can evaluate an expression and then solve it. I have got most of that down, but I ran into a little trouble because my teacher asked me to use a while loop to continuously ask for input and display the answer, and I am stuck on that. To end the program the user has to type in "quit" and I can't use system.exit() or any cheating thing like that, the program has to just run out of code. I have got most of that down too, but I am not able to find a why to return the expression in the Method MethodToReadInput(); Does anyone have any tips?

import java.util.*;

public class Calculator {
   public static void main(String[] args) {
      System.out.println("Welcome to the AP Computer Science calculator!!");
      System.out.println();
      System.out.println("Please use the following format in your expressions: (double)(space)(+,-,*,/...)(space)(double)");
      System.out.println("or: (symbol)(space)(double)");
      System.out.println();
      MethodToReadInput();
      MethodToTestInput(MethodToReadInput());

   }

   public static String MethodToReadInput() {
      Scanner kb = new Scanner(System.in);
      System.out.print("Enter an expression, or quit to exit: ");
      String expression = kb.nextLine();
      if (expression.equalsIgnoreCase("quit")) {
         System.out.println("Goodbye!");
      }
      else {
         return expression; 
      }
   } 
   public static void MethodToTestInput(String expression) {
      while (!expression.equalsIgnoreCase("quit")) {
          MethodToReadInput();
          MethodtoEvaluateInput(expression);
      }
      System.out.println("Goodbye!");
   }



   public static void MethodtoEvaluateInput(String expression) {
      if (OperatorFor2OperandExpressions(expression).equals("+")) {
         System.out.println(FirstOperandFor2OperandExpressions(expression) + " " + OperatorFor2OperandExpressions(expression) + " " + SecondOperandFor2OperandExpressions(expression) + " = " + (FirstOperandFor2OperandExpressions(expression) + SecondOperandFor2OperandExpressions(expression)));          
      }
      else if (OperatorFor2OperandExpressions(expression).equals("*")) {
         System.out.println(FirstOperandFor2OperandExpressions(expression) + " " + OperatorFor2OperandExpressions(expression) + " "  + SecondOperandFor2OperandExpressions(expression) + " = " + (FirstOperandFor2OperandExpressions(expression) * SecondOperandFor2OperandExpressions(expression)));
      }
      else if (OperatorFor2OperandExpressions(expression).equals("-")) {
         System.out.println(FirstOperandFor2OperandExpressions(expression) + " " + OperatorFor2OperandExpressions(expression) + " " + SecondOperandFor2OperandExpressions(expression) + " = " + (FirstOperandFor2OperandExpressions(expression) - SecondOperandFor2OperandExpressions(expression)));       
      }
      else if (OperatorFor2OperandExpressions(expression).equals("/")) {
         System.out.println(FirstOperandFor2OperandExpressions(expression) + " " + OperatorFor2OperandExpressions(expression) + " "  + SecondOperandFor2OperandExpressions(expression) + " = " + (FirstOperandFor2OperandExpressions(expression) / SecondOperandFor2OperandExpressions(expression)));
      }
      else if (OperatorFor2OperandExpressions(expression).equals("^")) {
         System.out.println(FirstOperandFor2OperandExpressions(expression) + " " + OperatorFor2OperandExpressions(expression) + " " + SecondOperandFor2OperandExpressions(expression) + " = " + Math.pow(FirstOperandFor2OperandExpressions(expression),SecondOperandFor2OperandExpressions(expression)));
      }
      else if (OperatorFor1OperandExpressions(expression).equals("|")) {
         System.out.println(OperatorFor1OperandExpressions(expression) + " " + OperandFor1OperatorExpressions(expression) + " = " + Math.abs(OperandFor1OperatorExpressions(expression)));
      }
      else if (OperatorFor1OperandExpressions(expression).equals("v")) {
         System.out.println(OperatorFor1OperandExpressions(expression) + " " + OperandFor1OperatorExpressions(expression) + " = " + Math.sqrt(OperandFor1OperatorExpressions(expression)));
      }
      else if (OperatorFor1OperandExpressions(expression).equals("~")) {
         double x = 0.0;
         System.out.println(OperatorFor1OperandExpressions(expression) + " " + OperandFor1OperatorExpressions(expression) + " = " + (Math.round(OperandFor1OperatorExpressions(expression))+ x));
      }
      else if (OperatorFor1OperandExpressions(expression).equals("s")) {
         System.out.println(OperatorFor1OperandExpressions(expression) + " " + OperandFor1OperatorExpressions(expression) + " = " + Math.sin(OperandFor1OperatorExpressions(expression)));
      }
      else if (OperatorFor1OperandExpressions(expression).equals("c")) {
         System.out.println(OperatorFor1OperandExpressions(expression) + " " + OperandFor1OperatorExpressions(expression) + " = " + Math.cos(OperandFor1OperatorExpressions(expression)));
      }
      else if (OperatorFor1OperandExpressions(expression).equals("t")) {
         System.out.println(OperatorFor1OperandExpressions(expression) + " " + OperandFor1OperatorExpressions(expression) + " = " + Math.tan(OperandFor1OperatorExpressions(expression))); 
      } 
   }

      public static double FirstOperandFor2OperandExpressions(String expression) {
         String[] tokens = expression.split(" ");
         String OperandOrOperator = tokens[0];
         double y = Double.parseDouble(OperandOrOperator);
         return y;         
   }
      public static double SecondOperandFor2OperandExpressions(String expression) {
         String[] tokens = expression.split(" ");
         String OperandOrOperator = tokens[2];
         double y = Double.parseDouble(OperandOrOperator);
         return y;
   }
       public static String OperatorFor2OperandExpressions(String expression) {
         String[] tokens = expression.split(" ");
         String OperandOrOperator = tokens[1];
         return OperandOrOperator;
   }
      public static String OperatorFor1OperandExpressions(String expression) {
         String[] tokens = expression.split(" ");
         String OperandOrOperator = tokens[0];
         return OperandOrOperator; 
   }
      public static double OperandFor1OperatorExpressions(String expression) {
         String[] tokens = expression.split(" ");
         String OperandOrOperator = tokens[1];
         double y = Double.parseDouble(OperandOrOperator);
         return y;  
   }           
}

Both, methods should begin with lower case, and, for example, instead of writing `underWaterExplisionBigWavesInTheOcean` you can write `tsunami`. — Maroun, Nov 10 '14 at 08:38
Methods generally are verbs as well, you're describing what it is instead of what it does. Instead try `testInput` or `readInput`. — ChiefTwoPencils, Nov 10 '14 at 09:05

score 1 · Answer 1 · answered Nov 10 '14 at 08:45

You need to put the MethodToReadInput and MethodtoEvaluateInput inside a loop. For example:

public static void main(String[] args)
{
    System.out.println("Welcome to the AP Computer Science calculator!!");
    System.out.println();
    System.out.println("Please use the following format in your expressions: (double)(space)(+,-,*,/...)(space)(double)");
    System.out.println("or: (symbol)(space)(double)");
    System.out.println();


    String input = MethodToReadInput();
    while (input != null)//exit the loop and the program when input is null
    {
        MethodtoEvaluateInput(input);//process the input
        input = MethodToReadInput();//ask the user for the next input
    }

}

public static String MethodToReadInput()
{
    Scanner kb = null;
    try
    {
        kb = new Scanner(System.in);
        System.out.print("Enter an expression, or quit to exit: ");
        String expression = kb.nextLine();
        if (expression.equalsIgnoreCase("quit"))
        {
            System.out.println("Goodbye!");
            return null;
        }
        else
        {
            return expression;
        }

    }
    finally
    {//always close the Scanner before leaving the method
        if (kb != null)
            kb.close();
    }
}

Also, you should follow the Java Naming Convention and use shorter names for your methods.

score 0 · Answer 2 · answered Nov 10 '14 at 08:41

Do this:

 public static String MethodToReadInput() {
      Scanner kb = new Scanner(System.in);
      System.out.print("Enter an expression, or quit to exit: ");
      String expression = kb.nextLine();
      if (expression.equalsIgnoreCase("quit")) {
         System.out.println("Goodbye!");
         return "";
      }
      else {
         return expression; 
      }

By returning an empty string you know what to look for when the user wants to exit. It needs to be an empty string that you return because your method is supposed to return a string. Also adding this return statement is needed because the compiler will complain otherwise because it is possible to reach the end of a non-void function (something that returns something) without actually reaching a return statement (so when you enter the if statement as you have it now). You must specify a return case for all possibilities if you specify a return type. In other words you must always return what you say you will.

the.wizard · Accepted Answer · 2014-11-10T09:09:02.330

Try to simplify your code, and use do-while-loop instead while-loop should produce a better code, do while will at least do one loop and then inspect the next condition before do the next loop, but while will inspect the condition first, if it is okay, it will do the loop. So here is the code:

public class Calculator {
    public static void main(String[] args) throws IOException {
        System.out.println("Welcome to the AP Computer Science calculator!!");
        System.out.println();
        System.out.println("Please use the following format in your expressions: (double)(space)(+,-,*,/...)(space)(double)");
        System.out.println("or: (symbol)(space)(double)");
        System.out.println();

        String expression = "";
        do {
            Scanner kb = new Scanner(System.in);
            System.out.print("Enter an expression, or quit to exit: ");
            expression = kb.nextLine();
            if (expression.equalsIgnoreCase("quit")) 
                System.out.println("Goodbye!");
            else 
                MethodtoEvaluateInput(expression);              
        } while (!expression.equalsIgnoreCase("quit"));
        inRn.close();
        inSw.close();
    }
}

score 0 · Answer 4 · answered Nov 10 '14 at 09:06

There are several things that should be fixed about this.

First, let's answer your actual question. You can have a number of choices.

You can just simply return whatever the user has input. In fact, you may not actually need the method for this. But anyway, if your method returns "quit", the while loop can check while ( ! expression.equals("quit") ) just as it does now.
You could return null. This indicates that "The expression is not an actual expression". Then your while could be while ( expression != null ) which is more efficient than string comparison.

But you have other design issues with your program:

You are calling the same methods again and again to retrieve the same things. Those methods split the string - a relatively heavy operation - again and again. You should probably just have a parseExpression() method that returns your tokens, and then something that tests whether these tokens represent a unary operator or a binary one. Something along the lines of:

String [] tokens = parseExpression( expression );

if ( isUnaryExpression( tokens ) ) {
     String operator = tokens[0];
     String operand = tokens[1];
     // Do something with operator and operand.
} else if ( isBinaryExpression( tokens ) ) {
     String operator = tokens[1];
     String operand1 = tokens[0];
     String operand2 = tokens[2];
     // Do something with operator and operands {
} else {
     System.err.println( "Bad expression!" );
}

You are calling MethodToReadInput twice from your main. This means it will Read one input, do nothing about it, and then read another one which will be passed to MethodToTestInput. Drop the first call, it's unnecessary.
In the cause of better encapsulation, the main method should actually not even call MethodToReadInput. It should become the responsibility of MethodToTestInput to call that method. So you just call MethodToTestInput() from main without passing a parameter at all.

So the structure should be:

main: Display introduction, call your looping method.
looping method: Call input method. Loop while returned expression is still an expression rather than "quit". Inside the loop, call expression handler method.
expression handler method: Call parseExpression() method, check what the tokens are, do the math.

Finally, about your naming issues:

In Java, we name only classes with an uppercase first letter. Constants are named with all capitals (words separated by underscore). Method names begin with a lowercase letter.
You don't name a method MethodThatDoesThis. You should name it doThis, instead. This makes reading your code easier because it actually describe what is happening. So I'd name the methods something like:
- The input method: getNextExpression
- The looping method: runCalculator, or doCalculatorMainLoop or something like that.
- The expression handler method: parseAndCalculate.
Or something along these lines.

Why isn't my program returning the value of my expression?

4 Answers4