C++

Forum

 
 
Need help with fixing a few of these errors

frustratedSEstudent (4)
My first two cases in the switch statement work smoothly, but the next two have a couple of errors. Can someone tell me how to fix them?

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
 
#include<iostream>
#include<cstdlib>
#include <string>
using namespace std;
int x;
int c = 0;
int operation()
{
	cout << "////////////////////////////MENU\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\" << endl;
	cout << "1 - Add Product" << endl;
	cout << "2 - View All" << endl;
	cout << "3 - Delete Product" << endl;
	cout << "4 - Update Product" << endl;
	
	cin >> x;
	return x;
}
struct products {
	char name[1];
	char description;
	char ID;
	int quantity;
	double price;

};
int main()
{
	products a[100];
Initialize:
	operation();
	switch (x)
	{
	case 1:
		cout << "You choose to add a product" << endl << endl;
		for (int i = (1 + c); i <= 100; i++)
		{
			if (i > 100)
			{
				cout << "You exceed the number of your products" << endl;
			}
			else
			{
				cout << "Please enter product ID" << endl;
				cin >> a[i].ID;
				cout << "Please enter the name of the product number " << a[i].ID << endl;
				cin >> a[i].name;
				cout << "Now please enter the quantity of " << a[i].name << endl;
				cin >> a[i].quantity;
				cout << "Now please enter the price of " << a[i].name << endl;
				cin >> a[i].price;
				cout << "Enter a description for " << a[i].name << endl;
				cin >> a[i].description;
				cout << "Product is successfully added" << endl << endl;
				c++;
				goto Initialize;
			}
		}
		break;
	case 2:
		cout << "You choose to view the products" << endl << endl;
		if (c == 0)
		{
			cout << "You have no products yet" << endl << endl;
			goto Initialize;
		}
		else {
			for (int i = 1; i <= c; i++)
			{
				cout << "Product ID is" << a[i].ID << endl;
				cout << " name is " << a[i].name << endl;
				cout << " quantity is " << a[i].quantity << endl;
				cout << " price is " << a[i].price << endl;
				cout << "Description of " << a[i].name << " is " << a[i].description << endl;
				cout << endl << endl;
			}
			goto Initialize;
		}
		break;
	case 3:
		cout << "You chose to delete product" << endl;
		for (int i = (1 + c); i <= 100; i++)
		{
			if (i > 100)
			{
				cout << "Please enter a number between 1 and 100" << endl;
			}
			else {
				cout << "Enter the product ID no." << endl;
				cin >> i;
				a[i].name = "null";
				a[i].quantity = "null";
				a[i].price = "0.0";
			}

			goto Initialize;
		}
		break;

	case 4:
		cout << "You chose to update product" << endl;
		for (int i = (1 + c); i <= 100; i++)
		{
			if (i > 100)
			{
				cout << "Please enter a number between 1 and 100" << endl;
			}
			else {
				char n;
				int q;
				double p;
				cout << "Enter the product ID no." << endl;
				cin >> i;
				cout << "Enter the updated product name" << endl;
				cin >> n;
				a[i].name = n;
				cout << "Enter the updated product quantity" << endl;
				cin >> q;
				a[i].quantity = q;
				cout << "Enter the updated product price" << endl;
				cin >> p;
				a[i].price = p;
			}

			goto Initialize;
		
		
	}
	return 0;
}
Last edited on
Flaze07 (231)
Please use codd tags..also...I'm pretty sure you havr heard that goto is a bad practice....

Handy Andy (5051)
Hello frustratedSEstudent,

PLEASE ALWAYS USE CODE TAGS (the <> formatting button) when posting code.
It makes it easier to read your code and also easier to respond to your post.
http://www.cplusplus.com/articles/jEywvCM9/
http://www.cplusplus.com/articles/z13hAqkS/
Hint: You can edit your post, highlight your code and press the <> formatting button.
You can use the preview button at the bottom to see how it looks.

At least your problems are minor.

1 In the struct you define char name[1]; that is a two character array and you are trying to put a four character sting into a two character array. Change to string name{""}; This will solve the first problem in case 3 in the else statement.

2. The next problem in case 3 is the line a[i].price = "0.0". You are trying to set double price to a string. remove the "" around the 0.0.

3. You will find that changing char name[1]; to string name{""}; will solve your problem in case 4.

4. Using goto statements is bad form and programming. You can eliminate the need for the gotos with a do/while loop around most of the code in main.

I have nor had time yet to test the fixes mentioned, but will shortly.

Hope that helps,

Andy
frustratedSEstudent (4)
Hello Handy Andy,
First of all, I implemented the use of code tags, I'm still pretty new to this site, so I didn't quite know about the code tags. Secondly, I tried the changes to my code that you suggested, and I no longer have errors in the code, although I didn't quite know how to replace the goto with a while loop. But, when running the code, and using the Add Product option, after entering the details, something strange happens, like it keeps repeating itself over and over a bunch of times. I'm not quite sure how to explain it, you'll see what I mean if you run the updated code:
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
 
#include<iostream>
#include<cstdlib>
#include <string>
using namespace std;
int x;
int c = 0;
int operation()
{
	cout << "////////////////////////////MENU\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\" << endl;
	cout << "1 - Add Product" << endl;
	cout << "2 - View Products" << endl;
	cout << "3 - Delete Product" << endl;
	cout << "4 - Update Product" << endl;
	cin >> x;
	return x;
}
struct products {
	string name{ "" };
	char description;
	char ID;
	int quantity;
	double price;

};
int main()
{
	products a[100];
Initialize:
	operation();
	switch (x)
	{
	case 1:
		cout << "You choose to add a product" << endl << endl;
		for (int i = (1 + c); i <= 100; i++)
		{
			if (i > 100)
			{
				cout << "You exceed the number of your products" << endl;
			}
			else
			{
				cout << "Please enter product ID" << endl;
				cin >> a[i].ID;
				cout << "Please enter the name of the product number " << a[i].ID << endl;
				cin >> a[i].name;
				cout << "Now please enter the quantity of " << a[i].name << endl;
				cin >> a[i].quantity;
				cout << "Now please enter the price of " << a[i].name << endl;
				cin >> a[i].price;
				cout << "Enter a description for " << a[i].name << endl;
				cin >> a[i].description;
				cout << "Product is successfully added" << endl << endl;
				c++;
				goto Initialize;
			}
		}
		break;
	case 2:
		cout << "You choose to view the products" << endl << endl;
		if (c == 0)
		{
			cout << "You have no products yet" << endl << endl;
			goto Initialize;
		}
		else {
			for (int i = 1; i <= c; i++)
			{
				cout << "Product ID is" << a[i].ID << endl;
				cout << " name is " << a[i].name << endl;
				cout << " quantity is " << a[i].quantity << endl;
				cout << " price is " << a[i].price << endl;
				cout << "Description of " << a[i].name << " is " << a[i].description << endl;
				cout << endl << endl;
			}
			goto Initialize;
		}
		break;
	case 3:
		cout << "You chose to delete product" << endl;
		for (int i = (1 + c); i <= 100; i++)
		{
			if (i > 100)
			{
				cout << "Please enter a number between 1 and 100" << endl;
			}
			else {
				cout << "Enter the product ID no." << endl;
				cin >> i;
				a[i].name = "null";
				a[i].quantity = 0;
				a[i].price = 0.0;
			}

			goto Initialize;
		}
		break;

	case 4:
		cout << "You chose to update product" << endl;
		for (int i = (1 + c); i <= 100; i++)
		{
			if (i > 100)
			{
				cout << "Please enter a number between 1 and 100" << endl;
			}
			else {
				char n;
				int q;
				double p;
				cout << "Enter the product ID no." << endl;
				cin >> i;
				cout << "Enter the updated product name" << endl;
				cin >> n;
				a[i].name = n;
				cout << "Enter the updated product quantity" << endl;
				cin >> q;
				a[i].quantity = q;
				cout << "Enter the updated product price" << endl;
				cin >> p;
				a[i].price = p;
			}

			goto Initialize;


		}
		return 0;
	}
}


And I'm really grateful for your help with this so far, I really do appreciate it :)
Last edited on
Handy Andy (5051)
Hello frustratedSEstudent,

Upon Testing your program this is what I found.

#include<cstdlib> I do not believe this header file is needed. Although I could be wrong. I did not use it and the program ran.

1
2
 
int x;
int c = 0;

These global variables are not needed. They can just as easily be local variables in the function "operation" and in main.

1
2
3
4
5
6
7
8
 
struct products
{
    char name[1];
    char description;
    char ID;
    int quantity;
    double price;
};

For the struct line 3 is a two element array with element zero holding one character and element one holding a "\0". I do not think that is what you want. Line 4 will only hold one character and the same with line 5. A better approach would be:
1
2
3
4
5
6
7
8
9
 
struct products
{
	string name;
	string description;
	string ID;
	int quantity;
	double price;

};


The function "int operation()" returns an "int", but it is never used. More on that later. The variable "x" is better defined in the function since it is being returned to main before the function ends.

In main you call the function "operation" which returns an "int", but you never use it. "x" can be defined in main and your function call would look like this: x = operation();. Now you can use "x" in your switch and it will work without the need for your global variable. Since the scope of the function "operation" and main are different "x" can be defined in each function without any conflict.

To eliminate the need for the goto statements I wrapped most of main in a do while loop. The idea is:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
bool cont{true};

do
{
    // Code here

    }  //  End of switch

	if (cont)
	{
		std::cout << "\n Do another: ";
		std::cin >> x;
	}

} while (cont);


The if statement is if the main menu choice was not to exit the program. I added choice 5 to the menu and case 5 to the switch to end the do/while loop and end the program. This should work, but I have not tested it yet because of other problems that need addressed first. I would also suggest a "default" case to catch any menu choice that is not correct. Although it would be better to catch that in the function "operation" before you return to main. Easier to check it in "operation" than in main.

In the switch I could only start with case 1 for now. The for loop starts out as: int i = (1 + c);. That works, but you are only using 99 out of 100 in your array. It would work better as: int i = (0 + c);. Later when "c" has a value > zero it would be useful to add to the array where you left off. And first time through you will start with element zero and use all 100 elements of the array.

In the "else" statements the lines that input "name" and "description" would work better with std::getline instead of the cin. cin will read until the first space or \n whichever comes first. And you will want to use this std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); after a std::cin >> and before a std::getline() to clear the "\n" left by the std::cin. This line of code needs the header file "limits" to work.

Other than adding a case 5 to the switch and the above mentioned if after the closing brace of the switch I have not looked at the rest if the program yet.

Hope that helps,

Andy

Edit: Your repeating loop comes from the for in case 1. There is no way out of it until "i" reaches 100. Sorry meant to mention that earlier.
Last edited on
AbstractionAnon (6954)
Line 9: \ is an escape character. That cout may not appear as you expect.

Lines 34, 66, 80, 100: C/C++ arrays are indexed from 0, not 1. The correct idiom for your for loops is:
1
2
 
 
  for (int i=0; i<100; i++)


Line 36,82,102: These if statements can never be true due to the termination condition of the for loops.

Lines 34,80,100: The for loop is unnecessary since you jump back to Initialize after adding a single entry.

Line 68: You make no check for a deleted product.

Line 88: You're changing your loop variable.

Line 108,113: n is a single character.

Handy Andy (5051)
Hello frustratedSEstudent,

As I started working on case 1 and started with the for loop I started to realize that it should not be there. As you have it setup if you have 5 entries to add you will have to enter another 95 before you find out that the subscript is out of bounds of the array. Also i <= 100 should be i < 100 as the array goes from 0 to 99 making 100 outside the bounds of your array which will cause the progam to crash. Of course since the for loop is not need its FYI.

I reworked the whole case 1 to use a while loop starting with: while (std::getline(std::cin, a[i].ID)). The afore mentioned cin.ignore... needs to be place in several place including the "operation" function.

For case 2 as AbstractionAnon said the for loop needs to start at zero not one. That one fix will prit the whole array. With that change case 2 works.

case 3 and 4 need a total rework. in case 3 the for loop should be: for (int i = 0; i < c; i++) which is useful when you compare something to either name or ID, so tou would know which element of the array to change. If you had used a vector instead of the array you could actualy delete that whole record with no poblem and the vector would resize itself making it easier to print out.

In case 4 the same idea fr case 3 is needed to find the correct element to update.

Still working on case 3 and 4. Not sure what I am goin to do yet.

Hope that helps,

Andy
Topic archived. No new replies allowed.