My gym_Membership source, C++, is it neat?

Give me some advice to make it cleaner and optimize the code. Also, I can't make the window stay after entering a letter, why?

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

using namespace std;
int main()
{
	const double packadge_A = 9.95, packadge_B = 14.95, packadge_C = 19.95;
	int users_packadge;
	double hours_used, hours_to_pay, hours_to_pay_in_money;
	
	cout << "-----------------------------------\n";
	cout << "Welcome to my ArtGym Membership\n";
	cout << "-----------------------------------\n\n";

	cout << "Select the plan you purshased:\n";
	cout << "1- Packadge A: $ 9.95.\n";
	cout << "2- Packadge B: $ 14.95.\n";
	cout << "3- Packadge C: $ 19.95.\n";
	cout << "4- Exit program... and go watch Naruto! \n\n";
	cout << "******|| Select an option from 1-4. ||*****\n\n";
	cin >> users_packadge;

	if (users_packadge == 1)

	{
		cout << "\nPackadge 1 was a great option, good choise! How many hours did you used? ^_^ \n\n";
		cin >> hours_used;

		if (hours_used > 10)
		{
			hours_to_pay = (hours_used - 10);
			hours_to_pay_in_money = (hours_to_pay * 2);

			cout << "\nYou are exeded in: " << hours_to_pay << " hours. You need to pay: $ " << hours_to_pay_in_money << " extra." << endl;
			double total_due;
			total_due = (hours_to_pay_in_money + 9.95);
			cout << "Your total amount due for this month is: " << total_due << endl;
		}
	}
	else if (users_packadge == 2)
	{
		cout << "\nPackadge 2 was a great option, good choise! How many hours did you used? ^_^ \n\n";
		cin >> hours_used;

		if (hours_used > 20)
		{
			hours_to_pay = (hours_used - 20);
			hours_to_pay_in_money = (hours_to_pay * 1);

			cout << "\nYou are exeded in: " << hours_to_pay << " hours. You need to pay: $ " << hours_to_pay_in_money << " extra." << endl;
			double total_due;
			total_due = (hours_to_pay_in_money + 14.95);
			cout << "Your total amount due for this month is: " << total_due << endl;
		}
	}
	else if (users_packadge == 3)
	{
		cout << "\nPackadge 3 was a great option, good choise! ^_^ \n\n";
		cout << "Your total amount due for this month is: $ " << packadge_C << ".";
	}

	else if (users_packadge == 4)
	{
		cout << "\nGood choice! Let's go watch Naruto together, the new episode came out!\n\n";
		cout << "Now the program will exit\n\n";
	}


	else
	{
		cout << "\n**----------------------------------------------**\n\n";
		cout << "That was not in the menu... Try again later. \n";
		cout << "Enter any key to continue...\n";
	}



	cin.ignore();
	cin.get();
	return 0;
}
1. Instead of all of the if( users_packadge == 1 ) type of code, I would have used a switch() statement.

2. Just because a line of code can be very long in an editor doesn't mean you should use a very long line. I would have placed your "You are exeded in" lines on two or more lines.

3. You have misspelled several words:
Packadge should be package
The grammar is wrong on "How many hours did you used?" It should be "How many hours did you use?"
choise should be choice
exeded should be exceeded (but your grammar is wrong. It would be better to say:
"You have exceeded " << hours_to_pay << " hours. You need to pay: $"

Other than those things, your code looks pretty good to me.

Yeah I didn't check for spelling errors lol

Thanks
Im also a beginner trying to learn c++. so excuse me if i cant see it, but what do you output to the user if the person dont exceed the gym time for the month ?
Actually, a switch statement is usually resolved to an if-else statement by the compiler, unless there are hundreds of conditions. In that case, however, the compiler would resolve it to JMP instructions.

1
2
3
const double packadge_A = 9.95, packadge_B = 14.95, packadge_C = 19.95;
	int users_packadge;
	double hours_used, hours_to_pay, hours_to_pay_in_money;


I don't know what that's supposed to represent, but you can start by throwing that into a data structure and call it "member_data" (or somthing similar; really whatever you want).

If you're going to store constants (like prices) make them defaults or somthing... or you can use template meta-programming to have compiler-time resolution and eliminate the variables alltogether.

You can write another function and call it "menu" or whatever you want. There's no need for everything to be within main, and it would make it easier for you to expand upon your work in the future, should you choose to do so.

I would choose a different approach to the code within the if-statements that act based on the "package" chosen. You might have a data structure called "package", and it stores an integer, price, and whatever else is associated with a "package". You can then declare constants within an array, and enumerate them as names:

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
//this is just pseudo:

typedef struct package package;

//package has constructor:  package::package(const double& price, const double& hour_limit)

//we try to avoid global constants by having a function return them instead.
inline std::vector<package> avail_packs()
{
    return std::vector<package>({
        package(5, 10),
        package(9.9, 20),
        package(15, 30)
    });
}

enum package_type
{
    package_a = 0,
    package_b
    package_c
};

//....

//easy peasy
std::cout<< "package A costs: $"<< avail_packs()[package_a].price<< std::endl;


Hope this helps.
@IWishIKnew: A switch statement resolves to test register and jump, but let's not split hairs on that. I say this because the OP is trying to learn C++. If your code has more than three conditions, a switch statement usually ends up being easier to read and maintain.

However, your comments about data structures and a menu function are great ideas!
Last edited on
While I agree with you, I usually prefer template metaprogramming. I only suggested enumerations because I thought this would be more confising for a beginner:

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

namespace
{
    template<unsigned long>
    struct const_int_type
    {
        static const unsigned long value;
    };
    
    template<unsigned long i> const unsigned long const_int_type<i>::value = i;
    
    
    inline const std::vector<std::string> my_const_vec()
    {
        return std::vector<std::string>{
                "Hello",
                "World"};
    }
    
}

namespace vec_indices
{
    typedef const_int_type<1> hello;
    typedef const_int_type<2> world;
}

int main()
{
    using std::cout;
    using std::endl;
    
    {
        using namespace vec_indices;
        cout<< my_const_vec()[hello::value]<< " "<< my_const_vec()[world::value]<< "!"<< endl;
    }
}


I also tend to disagree with you because when it comes to the addition and removal of objects to such an operation, it is as simple as declaring a new object. Provided a constructor, all can be inlined. Removal is just as simple. Remove the declaration of the constant and the compiler will tell you any illegal usages.
Your math is off.

total_due = (hours_to_pay_in_money + 9.95);

You should be multiplying the hours to pay by the package rate, not adding.
Topic archived. No new replies allowed.