-1

I am writing an evaluation code for polish notation. When I overcome ClassCastException I get EmptyStack and when I solve Emptystack I get ClassCast I'm in a loop. Here is how I wanted to evaluate the Polish notation:

First I get a string from the user then put it into a stack. For evaluating I use a second stack. Why? Because: An example string: "* + * + 1 2 + 3 4 5 6" First operation here is to add up 3 and 4 but what I'm gonna do with 5 and 6? I put them in another stack. Let's say stack2. I start popping until I found an operator, in this condition I pushed 6 5 4 3 into stack2 and found a plus operator. Then I pop 2 numbers which I pushed to stack2 (the top 2 numbers are 3 and 4), add them up, and push them to stack again. I should evaluate but seems like I am missing a point or this wasn't a good idea at first. Here is my code:

public class Main {

    public static void stack(String srt){  // this function puts the string in srt stack
        String arr[] = srt.split(" "); // I am recognizing if there is a 2 or more digit number
        int size= arr.length;                // with receiving prefix with " " btw every number and operator
        Stack stack= new Stack();
        for (String s : arr) {
           // System.out.println(s);
            stack.push(s);
        }
       // for (int i = 0; i < size; i++) {
       //     System.out.println(stack.pop()); // I checked to see if any 
                                               // problems first now I dont start this
      //  }

          evaluate(stack);             // then calls the evaluate function
    }

    public static void evaluate(Stack stack){
        Stack stack2= new Stack();
        stack2.push(stack.pop());// cuz of there cant be an operator there I pop first 2 number
        stack2.push(stack.pop());
        for (int i = 0; i < (stack.capacity()*2); i++) { // I had a hard time calculating how many times
                                                        // I should cycle through this for and leave it 2 times as stack capacity
            if(stack.peek().toString().equals("*") || stack.peek().toString().equals("-") || stack.peek().toString().equals("/") || stack.peek().toString().equals("+")){
                System.out.println("Checkpoint1");
//                System.out.println(stack2.peek());
                int s2= Integer.parseInt((String) stack2.pop());
                int s1= Integer.parseInt((String) stack2.pop());
                double s3;
                String c = (String) stack.pop();
                switch (c) {
                        case "+":
                            s3=s1+s2;
                            stack2.push(s3);
                        case "-":
                            s3=s1-s2;
                            stack2.push(s3);
                        case "*":
                            s3=s1*s2;
                            stack2.push(s3);
                        case "/":
                            s3=s1/s2;
                            stack2.push(s3);
                    }

            }else{
                System.out.println("Checkpoint 2");
                stack2.push(Integer.parseInt((String) stack.pop()));
//                System.out.println(stack.peek());
            }

            }

//        System.out.println(stack.peek());
//        System.out.println(stack2.peek());
        }



    public static void main(String[] args) {
        String examplestr ="* + * + 1 2 + 3 4 5 6";
        stack(examplestr);
    }
}

Thank you for your help!!!

user207421
  • 305,947
  • 44
  • 307
  • 483
  • 1
    Don't use the `Stack` type. It is a bad collection type, and you should use `ArrayDeque` instead. Also, use generics so you don't have to cast to the value for every call. You want the second stack to be tracking the numbers, so you would store and load `Integer`s, but in `"Checkpoint 2"` you are casting them to `String`s, and that would not work. The problem is you are not `pop`ing the stack in `"Checkpoint1"`, so it will run over and over again – Deadbeef Jan 06 '21 at 13:28
  • @Deadbeef Just for curiosity why is it consider bad? – dreamcrash Jan 06 '21 at 13:29
  • the [javadoc](https://docs.oracle.com/javase/8/docs/api/java/util/Stack.html) says to use `Deque`: A more complete and consistent set of LIFO stack operations is provided by the Deque interface and its implementations, which should be used in preference to this class. – Deadbeef Jan 06 '21 at 13:30
  • also take a look at this question: [Why is Java Vector (and Stack) class considered obsolete or deprecated?](https://stackoverflow.com/q/1386275/13854774) – Deadbeef Jan 06 '21 at 13:32
  • About `Deque`, I am still a student and they still teaching stack.We never heard Deque before. Considering the question you linked is 11 years old I wonder why we still using it even javadoc saying its better. Thank you for your help I going to work on my code @Deadbeef – Selçuk Başaran Jan 06 '21 at 13:53
  • First of all, in method stack(), you add all the operand and operators into stack. After that you pop them out, so stack becomes empty. The second problem with this approach, you have the values from the polish notation in revers order in stack - this does not really help. Third, this kind of problem requires only one stack and the expression itself - it's not worth doing it with two stacks. – vasile_t Jan 06 '21 at 13:54
  • @vasile_t, the first stack is the numbers and the operators that are yet to be evaluated, so they are not immediately popped off from the stack. Another stack is for tracking the current numbers. – Deadbeef Jan 06 '21 at 13:59
  • @vasile_t Sorry I forgot to put comment lines. I wanted to check if there is a problem when getting string into stack. I saw many examples evaluating with one stack but mostly they reverse the string before putting it into the stack. Cuz of this code must be working with multiple digit numbers I couldn't use these examples. About reverse order: We read notation from right to left so when I pop the first value from stack it's gonna be the most right value from the given string I couldn't see a problem there. – Selçuk Başaran Jan 06 '21 at 14:09
  • `* + * + 1 2 + 3 4 5 6 ` is prefix notation, not postfix. All you have to do is reverse it, then you can process it with a postfix algorithm. Only one stack required. Your question and your requirement are unclear. – user207421 Apr 12 '23 at 10:33
  • @user207421 Hi. Looking back at my code after much time I get it. At that time I found solutions online with only one stack but they were reversing the notation. I wanted to solve the polish as it is. Not with converting it to a postfix and solving the postfix. After all you are right. Thank you – Selçuk Başaran Apr 13 '23 at 12:24

1 Answers1

1

So turns out the switch-case doesn't operate correctly because I forgot a fundamental part "break". For EmptyStackException, I made a try-catch and put them inside so when it returns empty stack I know evaluation is done and print the result. BUT I still don't know how to deal with these little problems. It feels like I did not solve them properly. Gotta work more. Thanks.

Edit: I made some more adjustments and this is my final working code. I couldn't detect an error in the results yet.

Here is the working code;

import java.util.EmptyStackException;
import java.util.Stack;


public class Main2 {

    public static void stack(String srt) {     
        String arr[] = srt.split(" "); 
        int size = arr.length;                
        Stack stack = new Stack();
        for (int i = 0; i < size; i++) {
            if (arr[i].toString().equals("*") || arr[i].toString().equals("-") || arr[i].toString().equals("/") || arr[i].toString().equals("+"))
                stack.push(arr[i]);

            else
                stack.push(Double.parseDouble(arr[i]));

        }
//        for (int i = 0; i < size; i++) {
//            System.out.println(stack.pop()); 
//        }

        evaluate(stack);             
    }

    public static void evaluate(Stack stack) { 
        Stack stack1 = new Stack();
        stack1.push(stack.pop()); 
        stack1.push(stack.pop()); 

        for (int i = 0; i < stack1.capacity(); i++) {  
            try {
                if (stack.peek().toString().equals("*") || stack.peek().toString().equals("-") || stack.peek().toString().equals("/") || stack.peek().toString().equals("+")) {
//                    System.out.println("Checkpoint1");
                    double s1 = (double) stack1.pop();
                    double s2 = (double) stack1.pop();
                    double s3;
                    String operator = String.valueOf(stack.pop());
                    switch (operator) {
                        case "+":
                            s3 = s1 + s2;       
                            stack1.push(s3); 
                            break;            
                        case "-":               
                            s3 = s1 - s2;      
                            stack1.push(s3);   
                            break;     
                        case "*":              
                            s3 = s1 * s2;    
                            stack1.push(s3);    
                            break;
                        case "/":
                            s3 = s1 / s2;
                            stack1.push(s3);
                            break;
                    }

                } else {
//                    System.out.println("Checkpoint 2");
                    stack1.push(Double.parseDouble(String.valueOf(stack.pop())));
}

            } catch (EmptyStackException e) {
                System.out.println("Notasyonun sonucu: " + stack1.peek());

            }
        }
    }

    public static void main(String[] args) {
        String notasyon = scanner.nextLine();
        stack(notasyon);
    }
}
  • You need to put `break` statements at the end of each `case` in a `switch` statement, otherwise you get fall-through behaviour. See e.g. https://www.tutorialspoint.com/Java-fall-through-switch-statements – fishinear Jan 06 '21 at 18:00
  • Thanks. How did I manage to forget such a basic thing – Selçuk Başaran Jan 06 '21 at 18:07
  • An easier approach is to evaluate the values on the stack **while** going through the input. For each element from the input, push it on the stack. If the top two elements on the stack are numbers, then the third element has to be an operator. Pop all three, evaluate them and push the result on the stack. Examine the top two elements again and re-evaluate until they are not two numbers anymore. Then take the next element from the input, push it on the stack, and re-evaluate again. – fishinear Jan 06 '21 at 18:07
  • An alternative would be to process the complete input in reverse. Then it becomes post-fix notation, which is easier to handle: if it is a number push it on the stack. If it is an operator, pop the top two values, evaluate them, and push the result. The stack will only contain numbers in that case. – fishinear Jan 06 '21 at 18:12
  • @fishinear I can't be sure that the first three `pop` is gonna contain 2 numbers and an operator. As you can see my example string has 4 numbers back to back. Considering that we read this notation from right to left my program has to encounter 4 numbers without receiving any operator. + 3 4 5 6 in polish notation gives 7 5 6 instead of 3 4 11. – Selçuk Başaran Jan 06 '21 at 21:09
  • Thank you for your alternative idea. To be honest, I wanted to write this on my own and I couldn't get the idea. Reversing seemed more complicated. This was the easiest idea that I found @fishinear – Selçuk Başaran Jan 06 '21 at 21:14
  • If you evaluate while going through the input, then the stack will never contain + 3 4 5 6. After you have processed the first three symbols, the stack will contain + 3 4, which will then get evaluated to a 7, before processing the 5 and 6. – fishinear Jan 06 '21 at 21:44