feedback needed, please

cutwo (6)
Please help to fix code for project submission. I am trying to create a menu with a heap. I am using repl.it to code with three different files. Please review for feedback

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
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
 



#include <stdio.h>
#include <ctime>
#include <string.h>
#include <iostream>
#include <string>
using namespace std;

class Order {
    
public:
    
    string id;
    int year;
    bool warranty;
    
    Order(string id, int year, bool warranty) {
        this->id = id;
        this->year = year;
        this->warranty = warranty;
    }
    
    Order(){
        
    }
    bool operator>(Order other) {
        
        //warranty =1
        // no warranty < 6 = 2
        // no warranty > 5 = 3
        //Fix year!
        //time_t t = time(0);   // get time now
        //tm* now = localtime(&t);
        int yearNow = 2019 ;
        
        //Both have warranty.
        if(this->warranty && other.warranty) {
            
            //If year is  less than or equal to other.year than it has higher priority than other.
            if( yearNow -this->year < yearNow- other.year)
                return true;
           else
               return false;
        }
        
        //Both do not have warranty
        else if(!this->warranty && !other.warranty) {
            
            if(yearNow-this->year <yearNow- other.year)
                return true;
            else
                return false;
        }
        
        //This * only have warranty.
        else if(this->warranty) {
            return true;
        }
        else{
            return false;
        }
    }
};
class Heap {
    
public:
    Heap(int capacity) {
        this->capacity = capacity;
        size = 0;
        heap = new Order[capacity];
    }
    
    void push(Order*& order) {
        if(isFull())
            throw  "Heap is full cannot push more orders";
        
        heap[size] = *order; //Add incoming value to next available spot.
        fixHeapAbove(size);
        size++;
    }
    
  Order remove(){
        if(isEmpty())
            throw "Heap is Empty";
      
        Order deletedValue = heap[0];
        
        //Take right most leaf and replace the value we want to delete
        heap[0] = heap[size-1];
        
        //Only this if  necessary for priority queu
        fixHeapBelow(0);
        size--;
        
        return deletedValue;
    }
    
    Order peak(){
        if(isEmpty())
            throw "Heap is empty\n";
        
        return heap[0];
    }
    
    bool isFull(){
       return size == capacity;
    }
    
    void display(){
        for(int i = 0; i<size; i++)
            cout << heap[i].id << ", ";
        
        cout << endl;
    }
    
private:
    int getParent(int i){
        return (i-1)/2;
    }
    
    
    //When inserting value is bigger than its parent
    void fixHeapAbove(int i){
       
        Order newOrder = heap[i];
        
        //Note: the symbol > has been overloaded for the Order class.
        
        //Check if the newOrder is greater than parent
        //if it is we swapt then rinse and repeat.
        while (i > 0 && newOrder > heap[getParent(i)]) {
            heap[i] = heap[getParent(i)];
            i = getParent(i);
        }
        
        heap[i] = newOrder;
    }
    
    //when replacement value is less than its parent: no need to fix heap above
    //we are going to compare with its two children. and swapt with the largest
    //of the two.
    void fixHeapBelow(int i) {
        int childToSwap;
        
        while(i < size) {
            
            //Get children.
            int leftChild = getLeftChild(i);
            int rightChild = getRightChild(i);
            
            if(leftChild < size) { //Comparing with left child because that's first children. if
                                   // it doesn't have a left child than its a leaf. and we are done.
                
                if(rightChild > size-1) { //Does not have a right child
                    childToSwap = leftChild;
                }
                else { //If it has two children.
                    //Swapt with the biggest child
                    childToSwap = (heap[leftChild] > heap[rightChild] ? leftChild: rightChild);
                }
                
                //We need to swapt if parent is less than child
                if( heap[childToSwap] > heap[i]) {
                    Order tmp =  heap[i];
                    heap[i] = heap[childToSwap];
                    heap[childToSwap] = tmp;
                } else { //Otherwise get out of loop. NO work need to be done.
                    break;
                }
                
                i = childToSwap;
            }
            else {
                break;
            }
        }
    }
    
   bool isEmpty(){
        return size == 0;
    }
    
    int getLeftChild(int i) {
        return 2*i + 1;
    }

    int getRightChild(int i) {
        return 2*i + 2;
    }
private:
    int size;
    int capacity;
    Order* heap;
};



int main(){
  Heap heap(100);
    
    do {
cout<<"1.Insert new Record\n";
cout<<"2.Service Record\n";
cout<<"3.Display queue\n";
cout<<"4.Exit\n";
cout<<"Enter your choice : "<<endl;
    
    
    string id;
    string input;
    for(int i = 0; i<3; i++) {
        //Format input
        string temp = "";
        int count = 0;
        Order *newOrder = new Order();
        for(int j = 0; j<input.length(); j++){
            
            if(input[j] != '-')
                temp += input[j];
            else {
                if(count == 0) {
                    newOrder->id = temp;
                } else if(count == 1){
                    newOrder->year = stoi(temp);
                } else if(count == 2){
                    if(temp == "y")
                        newOrder->warranty = true;
                    else
                        newOrder->warranty = false;
                }
                count++;
                temp = "";
            }
        }

        
getline(cin,input);
switch(input) {
case 1:
cout<<"Input the customerid and year of service"<<endl;
cin>>customerid;
cin>>year;
cout<<"Enter its warranty status in y/n : "<<endl;
cin>>warranty;
Order();
break;

case 2:
remove();
break;

case 3:
display();
break;

case 4:
count();
cout<<"Exit & end-of-file"<<endl;
break;

default :
cout<<"Wrong choice\n";
}
}while(input != 4);

}
    heap.push(newOrder);
    }
    
    heap.display();
    return 0;
Last edited on
Handy Andy (5051)
Hello cutwo,

I am trying to create a menu with a heap.


This statement is rather gormless. Are you wanting to store the menu on the heap for later use? Or, as the code suggests, it is a menu driven program that uses the heap for storage?

I am not use to "repl.it" and what it is like, but if your code is any indication I would suggest getting your own IDE.

Either you did not compile this code or the compiler at "repl,it" is not very good. because the function "main" has several errors.

From the top:

your include files
1
2
3
4
5
6
7
8
9
10
11
 

#include <iostream>
#include <string>

#include <ctime>

//#include <stdio.h>
//#include <string.h>

using namespace std;  // <--- Best not to use.

The first two lines are what I look at as being what you are most likely to put into any program. Next would be any header files that are specific to the program, in this case "ctime". The last two header files are C header files. "stdio.h" is most likely included through "iostream" and "string.h" is not even used in this program". When I commented them out it made no difference.

The last line the comment says it all. Many posts have been written here and elsewhere on this subject. Do a search if you are interested.

The class "Order' everything is "public" this tends to circumvent the point of a class. This way any line of code can change these variables and you may not know it happened.

In the class "Heap" you start with a "public" section of functions followed be a "private" section of functions. I am not sure why you want some of the functions private and since I have not been able to run the program yet I do not know how it will work.

To cover the problems in "main":

First I will point yo to a previous by salem c http://www.cplusplus.com/forum/beginner/250857/#msg1104697 For what little I looked at in his link I tend to prefer the "Allman" style of coding. I feel it makes the code easier to read and follow. Along with the proper indentation it all helps. As an example I offer this:
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
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
 
int main()
{
	Heap heap(100);

	do
	{
		cout << "1.Insert new Record\n";
		cout << "2.Service Record\n";
		cout << "3.Display queue\n";
		cout << "4.Exit\n";
		cout << "Enter your choice : " << endl;

		string id;
		string input;

		for (int i = 0; i < 3; i++)
		{
			//Format input
			string temp = "";
			int count = 0;
			Order *newOrder = new Order();

			for (int j = 0; j < input.length(); j++)
			{
				if (input[j] != '-')
					temp += input[j];
				else
				{
					if (count == 0)
					{
						newOrder->id = temp;
					}
					else if (count == 1)
					{
						newOrder->year = stoi(temp);
					}
					else if (count == 2)
					{
						if (temp == "y")
							newOrder->warranty = true;
						else
							newOrder->warranty = false;
					}
					count++;
					temp = "";
				}  //  End of else.

			}  //  End of second for loop.


			getline(cin, input);

			switch (input)
			{
			case 1:
				cout << "Input the customerid and year of service" << endl;
				cin >> customerid;
				cin >> year;
				cout << "Enter its warranty status in y/n : " << endl;
				cin >> warranty;
				Order();
				break;

			case 2:
				remove();
				break;

			case 3:
				display{};
				break;

			case 4:
				count;
				cout << "Exit & end-of-file" << endl;
				break;

			default:
				cout << "Wrong choice\n";
			}  //  End of switch.

		}while (input != 4);  //  End of first or outer for loop. What is the while loop doing here?

	}  //  End of do/while loop. Missing while condition.
	heap.push(newOrder);
}  //  End of main.

// <--- Should these be inside "main"?
//heap.display();
//return 0;

Keep in mind that the indentation here is a bit exaggerated and your IDE should not look like what is posted here.

If you can put a comment after the closing } it helps now and in the future along with having the {}s line up in the same column.

Once I changed thing around I noticed that line 81 has the while and condition at the end of a for loop when it needs to be on line 83 to go with the do loop.

The very last two lines of code I believe should be above line 85 and the closing } of "main".

On line 24 "j" is defined as an "int", but "input.length()" will return a "size_t" or "unsigned int" which creates a type mismatch. Enough to generate a warning, but not enough to stop the compile or running of the program. It may not always be necessary, but it helps when the types match.

On line 53 you are trying to switch on a string and that does not work. You will find "switch" at the bottom, but the whole page is worth reviewing. http://www.cplusplus.com/doc/tutorial/control/

The condition of a switch needs to be an integral type meaning some type of an "int" or a "char". A string may contain more than one character which does not work.

Starting with line 57 "customerid", "year" and "warranty" are all undefined variables. Did you mean to use variables from the class "Order" or did you forget to define these variables?

Line 65 the function call to "remove" is missing a parameter.

Line 69 "display" id undefined. Did you want to use a function from a class or do you need to write this function?

Then each case statement is based on a number, but you are trying to switch on a string, so they do not match.

The while statement on line 81, which needs to be after the closing brace } of line 83, is trying to compare a "string" to an "int" and this will not work.

I refer you back to http://www.cplusplus.com/forum/beginner/251348/ and http://www.cplusplus.com/forum/beginner/250857/

All the responses from MikeStgt, Grey Wolf, Repeater and salem c although they may not directly apply to this program they are worth the time to learn from.

Hope that helps,

Andy
Topic archived. No new replies allowed.