C++

Forum

 
Too many lines?

Vivec (4)
Hello everyone!

Just started learning C++ and thought i`d have a go at a vending machine.

Although its just 46 lines, im thinking theres a simpler way of making it work? Or isnt there?

Without knowing what part of the language to look into next, im stuck. =p

Any pointers?

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 "stdafx.h"
#include <iostream>

using namespace std;

int selection(); // declares selection to invalidSelect

void invalidSelection() // asks user to sober up and calls selection again
{
	cout << "I dont have that, please try again when sober!" << endl;
	selection(); // returns to selection for another try
}
int selection()			// shows user selection of sodas and outputs the choice
{	
	cout << "Please choose your beverage # and press enter!\n" <<endl;
	cout << "#1.Cola\n#2.Pepsi\n#3.Fanta\n#4.Sprite\n#5.Urge\n"<<endl;
	cout << "#";
	int soda;
	cin >> soda;
		if (soda==1) cout << "You selected " << soda << endl;
		else if (soda==2) cout << "You selected " << soda <<endl;
		else if (soda==3) cout << "You selected " << soda <<endl;
		else if (soda==4) cout << "You selected " << soda << endl;
		else if (soda==5) cout << "You selected " << soda << endl;
		else invalidSelection();
		return 0;
}
int calculate(int cost=5) //calculates change and outputs cost
{	
		cout << "This beverage costs 5 credits\nYour payment: " << endl;	
		int pay;
		cin >> pay;
		int change = (pay-cost);		// input minus cost of soda
		if (pay < cost) 
			cout << "No soda for you!\n"<< endl; // if user enters under 5 credits // 
		else 
			cout << "Your change: " << change << " credits.\nPlease come again!" << endl;
		selection();
		return 0;
}
int main()
{
	selection();		// starts selection
	calculate();		 // starts calculate
	return 0;	
}
Owain (149)
Well, just by studying it at a glance, I would say that a 'switch' statement wouldent be amiss in your code. You should replace all your if...else if...else if.... chunks of code with one. But they dont even need to be there. Just say that if soda is more than 5 then execute invalidSelection()
Last edited on
gelatine (256)
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
 
#include "stdafx.h"
#include <iostream>

using namespace std;

int selection()			// shows user selection of sodas and outputs the choice
{	
	cout << "Please choose your beverage # and press enter!\n" <<endl;
	cout << "#1.Cola\n#2.Pepsi\n#3.Fanta\n#4.Sprite\n#5.Urge\n"<<endl;
	cout << "#";
	int soda=0;
while (soda<=0 && soda>5) {
	cin >> soda; 
		if (soda==1) cout << "You selected Cola" << endl;
		else if (soda==2) cout << "You selected Pepsi"<<endl;
		else if (soda==3) cout << "You selected Fanta" <<endl;
		else if (soda==4) cout << "You selected Sprite" << endl;
		else if (soda==5) cout << "You selected Urge" << endl;
		else {cout << "I dont have that, please try again when sober!" << endl;}
}
		return 0;
}
int calculate(int cost=5) //calculates change and outputs cost
{	
		cout << "This beverage costs 5 credits\nYour payment: " << endl;	
		int pay;
		cin >> pay;
		float change = (pay-cost);		// input minus cost of soda
		if (pay < cost) 
			cout << "No soda for you!\n"<< endl; // if user enters under 5 credits // 
		else 
			cout << "Your change: " << change << " credits.\nPlease come again!" << endl;
		return 0;
}
int main()
{
	selection();		// starts selection
	calculate();		 // starts calculate
	return 0;	
}

i made it a bit shorter.. but i don't see why would you want it to be short? if you want you can just put it all on 1 line but thats very bad...
Last edited on
Owain (149)
@gelatine I think he just wants his code to be more efficent, as opposed to literly using less lines. Am I right Vivec?
Last edited on
Vivec (4)
That was fast.
Yep, not literally less lines, just more compact, like newer cars.....
Ill have a look at switch statements Owain, thx!
If I were to say that if soda is more than 5 then execute invalidSelection() , wouldnt that enable the user to type 0 and break the machine?
I know, doesnt make much sense having an invalid key on the machine.. but still. =p
stoffe1100 (137)
That is very well structured code for a beginner! But as previous reply said. Switch statements is not a bad idea!
vlad from moscow (6539)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
int selection(); // declares selection to invalidSelect

void invalidSelection() // asks user to sober up and calls selection again
{
	cout << "I dont have that, please try again when sober!" << endl;
	selection(); // returns to selection for another try
}
int selection()			// shows user selection of sodas and outputs the choice
{	
	cout << "Please choose your beverage # and press enter!\n" <<endl;
	cout << "#1.Cola\n#2.Pepsi\n#3.Fanta\n#4.Sprite\n#5.Urge\n"<<endl;
	cout << "#";
	int soda;
	cin >> soda;
		if (soda==1) cout << "You selected " << soda << endl;
		else if (soda==2) cout << "You selected " << soda <<endl;
		else if (soda==3) cout << "You selected " << soda <<endl;
		else if (soda==4) cout << "You selected " << soda << endl;
		else if (soda==5) cout << "You selected " << soda << endl;
		else invalidSelection();
		return 0;
}


This is very bad style then these two functions refer each other. And I do not see any requirements in the separate function invalidSelection. Also it is not cllear why selection does have return type int when it always returns 0?! Maybe it should return the value of variable soda?

I would write the following way

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
 

int selection()			// shows user selection of sodas and outputs the choice
{	
	int soda;
 
	do
	{

		cout << "Please choose your beverage # and press enter!\n" <<endl;
		cout << "#1.Cola\n#2.Pepsi\n#3.Fanta\n#4.Sprite\n#5.Urge\n"<<endl;
		cout << "#";

		cin >> soda;
		
		switch ( soda )
		{
			case 1:
				cout << "You selected " << soda << endl;
				break;
			case 2:
				cout << "You selected " << soda <<endl;
				break;
			case 3: 
				cout << "You selected " << soda <<endl;
				break;
			case 4:
				 cout << "You selected " << soda << endl;
				break;
			case 5: 
				cout << "You selected " << soda << endl;
				break;
			default:
				cout << "I dont have that, please try again when sober!" << endl;
				break;
		}
	} while ( soda > 5 || soda < 1 );

	return soda;
}

Last edited on
Vivec (4)
Thats a very good example of switch statements put to use! do/while statements aswell. Didnt know about those yet. Thanks vlad!

Why is it bad to refer between functions? Because its out of main()?
When you write return soda; that returns only the user input to main? Or what?
vlad from moscow (6539)
You made recursive calls of the functions iteslf. For example

selection()
invalidSelection() // in case a wrong input
selection()
invalidSelection() // in case other wring input
and so on.

That is the stack will be filled with arguments and local variables of these function that can result in the program abort.

As for return value there is no any sense to return 0 from the function. It would be more logically consistent to declare it as void because its return value is not used.
I kept the return type by returning more useful value - user input, that can be analyzed further in main.
Lowest0ne (1536)
Thats a very good example of switch statements put to use!

Hmm, the reason for the break; is that otherwise the switch would continue into the next case. It is therefor possible to write:
1
2
3
4
5
6
7
8
9
10
11
12
13
 
switch ( soda )
{
  case 1:
  case 2:
  case 3: 
  case 4:
  case 5: 
    cout << "You selected " << soda << endl;
    break;
  default:
    cout << "I dont have that, please try again when sober!" << endl;
    break;  // No need for this in the default condition
}


I know, doesnt make much sense having an invalid key on the machine


Vending machines generally use rows and columns, the buttons being "A5" or something. Maybe for fun you could incorporate this.
Last edited on
ramanajan (2)
Of course it's always a good idea to stay away from "magic" numbers in code. So perhaps a clearer way to code this would be:

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
 
#include <iostream>

using namespace std;

typedef enum
{
    NOT_A_COLA,
    // Insert new soda types here
    COLA,
    PEPSI,
    FANTA,
    SPRITE,
    URGE,
    // New soda types must be before here
    INVALID
} SODA_TYPE;


const char * SODA_NAMES[INVALID] =
{
    "", // Need this since the array is zero based by default
    "Cola",
    "Pepsi",
    "Fanta",
    "Sprite",
    "Urge"
};

#define FIRST_SODA (NOT_A_COLA + 1)
#define LAST_SODA (INVALID - 1)

SODA_TYPE selection();

int main()
{
    SODA_TYPE soda;

    while (true)
    {
        soda = selection();
    }   
    return (int) soda;
}

SODA_TYPE selection()			// shows user selection of sodas and outputs the choice
{	
    int soda;

    do
    {

        cout << "Please choose your beverage # and press enter!\n" <<endl;
        for (int i = FIRST_SODA; i <= LAST_SODA; i++)
        {
            cout << "#" << i << " " << SODA_NAMES[i] << endl;
        }
        cout << endl << "#";

        cin >> soda;

        switch ( (SODA_TYPE)soda )
        {
        case COLA:
        case PEPSI:
        case FANTA: 
        case SPRITE:
        case URGE: 
            cout << endl << "You selected " << SODA_NAMES[soda] << "." << endl;
            break;
        default:
            cout << "I dont have that, please try again when sober!" << endl << endl;
            break;
        }
    } while ( soda > LAST_SODA || soda < FIRST_SODA );

    return (SODA_TYPE)soda;
}


Of course this may be overkill for such a short program. However, in some ways it is "better" since it would be easier to add/change sodas and the cases are very clear as to what they refer to. In coding "better" is subjective and depends in the end on what the code is for.
Vivec (4)
Wow, I really appreciate all the inputs guys! Very helpful and good learning!

Ill do some homework on that new stuff you used, ramanajan. Not quite grasping how it works, just by reading through it. Especially the for statement!
Topic archived. No new replies allowed.