How can I make this program neater?

For one of my assignments, I needed to make an RPN calculator by using linked lists. I showed my project to my teacher looking for feed back, but he simply said that it could be neater, and didn't describe how. Is there anything I can change to this program to make it look more organized? (In terms of input and/or output)

I know there aren't comments here, but I'm more curious about the code.

Header file:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36

#ifndef RPN_H_INCLUDED
#define RPN_H_INCLUDED

#include <iostream>
#include <string>
#include <sstream>
using namespace std;

struct NODE {
       float num;
       NODE *next;
};

class stack {
    private:
      NODE *head;
      
    public:
      stack();
      void push(float);
      float pop();
      int nElements();
      float display();    
};

class RPN: public stack {
    public:
      void add();
      void subtract();
      void multiply();
      void divide();
};

#endif


.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66

#include "rpn.h"

stack::stack(){
     head = NULL;
}

void stack::push(float a_number){
     NODE *temp = new NODE;
     if (temp){
        temp->num = a_number;
        temp->next = head;
        head = temp;
     }
}

float stack::pop(){
      float number=0;
      if (!head)
         return 0;
      else {
           NODE *temp=head;
           number = head->num;
           head = temp->next;
           delete temp;
      }
      return number;
}

int stack::nElements(){
    int counter=0;
    for (NODE *node=head; node; node=node->next)
        counter++;
    return counter;
}

float stack::display(){
      if (nElements() > 0){
         float temp = pop();
         cout << temp << endl;
         push(temp);
      }
}



void RPN::add(){
     if (nElements()>=2)
        push(pop() + pop());
}

void RPN::subtract(){
     if (nElements()>=2)
        push(0 - pop() + pop());
}

void RPN::multiply(){
     if (nElements()>=2)
        push(pop() * pop());
}

void RPN::divide(){
     if (nElements()>=2)
        push(1/pop() * pop());
}


.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
#include "rpn.h";
#include "rpn.cpp";

#include <iostream>
#include <string>
#include <sstream>

int main(){

    RPN calculator;
    string input="";
    string valid="0123456789-.";
    float value=0;
    char operation='\0';
    
    while (operation != 'Q' && operation != 'q'){
          getline(cin, input);
          if (input.length()==1 && input[0]>'9' || input[0]<'0'){
             operation = input[0];
             switch (operation){
                    case '+': calculator.add();
                              calculator.display();
                              break;
                    case '-': calculator.subtract();
                              calculator.display();
                              break;
                    case '*': calculator.multiply();
                              calculator.display();
                              break;
                    case '/': calculator.divide();
                              calculator.display();
                              break;
                    case 'q':
                    case 'Q': break;
                    default: cout << "Invalid input" << endl;
             }                       
          }
          else {
               
               stringstream(input) >> value;
               calculator.push(value);
          }
    }
    
    return 0;
}
Last edited on
You could start by removing the superfluous #include s and using namespace std;s.

FYI you posted the header twice.
Topic archived. No new replies allowed.