Optimizing code

Can someone help optimize my code?
It seems kinda long and messy, especially since i tried to make it bug proof.

I really wanna learn to make my code run fast and take less memory.

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
#include <iostream>
using namespace std;


//Function Declarations
void seating (int,int,char seat[13][13]);

int main ()
{
    int k,l,z=0,x=0;
    char y=0,ttype,sent=y,seat[13][13];

    
//Initialize the array; All empty seats.
   for (int i=12;i>=0;--i)
   {
       for (int j=5;j>=0;--j)
       {
           seat[i][j]='*';
       }
   }

//Get Ticket Type
   cout<<"Welcome.\n";
error0:
   cout<<"Please enter your ticket type \n[F for First Class ; B for Business Class; E for Economy Class] \n[F/B/E] : ";
   cin >> ttype;
   cout <<"\nYou have ";
   
   switch (ttype)
   {
          case 'F' :
          case 'f' :
            {
               cout<<"first class ticket.";
               k=0;
               l=1;
               break;
            }
          case 'B' :
          case 'b' :
             {
               cout<<"business class ticket.";
               k=2;
               l=6;
               break;
             }
          case 'E' :
          case 'e' :
             {  
               cout<<"economy class ticket.";
               k=7;
               l=12;
               break;
             }
          default:
             {
                system ("cls");
                cout<<"\nYou have entered an invalid ticket type. Please try again.\n";
                goto error0;
             }
   }

//Display seats available
error1:
     cout << "\n'*' are seats available to you.";
     seating (k,l,seat);

//Get desired seat (row)
     cout<< "Enter your desired seat: [eg 1A] : ";
     cin >>x;
     x-=1;
     if (cin.fail()||x<k||x>l)
     {   
         cin.clear();             //clear error flag created by cin.fail... not too sure how it works.. found online
         cin.ignore();            
         //cin.fail leaves the input in memory and the bad input will be used again unless cin.ignore is used, came with cin.clear
error2:
         system ("cls");
         cout<<"The seat you have chosen is invalid or unavailable. Please choose another.";
         goto error1;        
     }
     
     
//Get desired seat (column)     
     cin>>y;
     
     switch (y)
   {
          case 'A' :
          case 'a' :
            {
               z=0;
               break;
            }
          case 'B' :
          case 'b' :
             {
               z=1;
               break;
             }
          case 'C' :
          case 'c' :
             {  
               z=2;
               break;
             }
          case 'D' :
          case 'd' :
             {  
               z=3;
               break;
             }
          case 'E' :
          case 'e' :
             {  
               z=4;
               break;
             }
          case 'F' :
          case 'f' :
             {  
               z=5;
               break;
             }
          default:
          {goto error2;}
   }
// Occupy seat
   if(seat[x][z]!='X')
   {seat[x][z]='X';}
   else
   {goto error2;}
   
//comfirmation report
   system ("cls");
   cout<<"\nYou seat["<<x+1<<static_cast<char>(toupper(y))<<"] has been succesfully booked.\nThis is the current seating for your flight.";
   seating(0,12,seat);
 
//continue?
error3:
   cin.ignore(1000,'\n');
   cout<<"\nDo you wish to continue?\n[y/n] : ";
   cin>>sent;

   if (sent=='Y'||sent=='y')
   {
      system ("cls");
      cout<<"You have chosen to continue.\n";
      goto error0;
   }
   else if (sent=='n'||sent=='N')
   {
        cout <<"Thank you for using our service. Have a nice day.\n";
   }
   else
   {
       cout<< "\nInvalid selection.";
       goto error3;
   }


system ("pause");
return 0;
}


//Function Definitions
void seating (int k,int l,char seat[13][13])
{
     cout <<"\n\t\tA\tB\tC\t\tD\tE\tF\n";
     for (int i=k;i<=l;++i)
      {
           
           if (i<9)
           {cout << "Row ["<<i+1<<"]\t\t";}
           else if (i>=9)
           {cout << "Row ["<<i+1<<"]\t";}
           
           
           for (int j=0;j<=5;++j)
           {
               cout<<seat[i][j]<<"\t";
               if (j==2)
               {cout<<"\t";}
           }
           cout<<endl;
      }
}   
This is an assignment for my university btw..
i already used a few function not covered by the lecturer yet.
Namely, goto, cin.clear, cin.ignore
You are using goto in a bad way, you should split your program in functions.
Don't use system():
- http://www.cplusplus.com/forum/articles/11153/
- http://www.cplusplus.com/forum/articles/10515/
- http://www.cplusplus.com/forum/articles/7312/
thanks for the input.
I started with functions, lol.

But it seemed easier to just put it all in main since referencing gets messy for me, and global variables... im just not very comfortable with that. Also, you cant goto main from functions.

goto is new to me, so im not sure how to use it.

I'll try and split it up again.
Imma stick with system a while, maybe until this course is over.
Last edited on
Why are you taking a course on learning to program, but refusing all good advice on learning programming?

Don't use goto -- use functions.
Avoid global variables. Use function arguments.
Don't use system(). You don't need it.
eh, im not refusing them.

i said i would try splitting them up again...
i didnt know goto was a bad thing, it seemed so... convenient
before this i used recursion.

Im using system cause its in the lecture notes...

Yeah, functions and a loop are better.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
bool done = false;
while (!done)
  {
  char ticket_type;

  cout << "blah blah blah";
  cin >> ticket_type;

  switch (toupper( ticket_type ))
    {
    case 'Q': done = true; break;
    case 'F': get_first_class_ticket(); break;
    ...
    default: cout << "What?\n";
    }
  }

This treelike kind of stuff is better, avoids recursion, and is easier to read, maintain, and modify.

As for system(), just know that it is bad, but for now follow your professor's directions.

Hope this helps.
oh wow, i never thought of using toupper for switch.
BTW can someone tell me why goto is bad?
I searched around, seems the only problem is it is harder to read, and is relatively new
Last edited on
New?!

goto is old -- it existed before such handy things as structured programming.

Keep searching. You've missed all the useful conversations.
http://www.cplusplus.com/forum/beginner/3889/ (don't forget to follow the links)

owh.. haha. Thanks Duoas

Anyway i removed all the goto.
Doesnt look much neater tho
Edit: Better.. thanks bazzy
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
#include <iostream>
using namespace std;


//Function Declarations
void seating   (int,int,char seat[13][13]);
void tictype   (int &, int &);
int  converty  (char);
char option    ();

int main ()
{
    int k,l,z=0,x=0,sent3=0;
    char y=0,sent='Y',seat[13][13];

    for (int i=12;i>=0;--i)              //Initialize the array; All empty seats.
    {   
        for (int j=5;j>=0;--j)
        {    seat[i][j]='*';    }
    }
    cout<<"Welcome.\n";
   
    do{
         tictype (k,l);                                              //Get Ticket Type                      
         do{
                 if(sent3==1)
                 {
                     system ("cls");
                     cout<<"The seat you have chosen is invalid or unavailable. Please choose another.";
                 }
                 sent3=0;                                            //Display seats available
                 cout << "\n'*' are seats available to you.";
                 seating (k,l,seat);
        
                 cout<< "Enter your desired seat: [eg 1A] : ";       //Get desired seat (row)
                 cin >>x;
                 x-=1;
                 if (cin.fail()||x<k||x>l)
                 {   
                     cin.clear();             //clear error flag created by cin.fail? found online
                     cin.ignore(1000,'\n');   //Clears bad input from memory, preventing it from being reused.
                     sent3=1;
                 }
        
                 if (sent3!=1)                                        //Get desired seat (column)
                 {
                     cin>>y;
                     z=converty (y);                                  //convert y to int

                     if(seat[x][z]=='*')                              // Occupy seat
                     {seat[x][z]='O';}
                     else
                     {sent3=1;}
                 }
        }while(sent3==1);
   
        system ("cls");                                            //comfirmation report
        cout<<"\nYou seat["<<x+1<<static_cast<char>(toupper(y))<<"] has been succesfully booked.\nThis is the current seating for your flight.\nYour seat is marked by 'O'";
        seating(0,12,seat);
        seat[x][z]='X';
    
        sent=option();                                             //continue?
        
    }while(sent!='N'&&sent!='n');

system ("pause");
return 0;
}


//Function Definitions
void seating (int k,int l,char seat[13][13])
{
     cout <<"\n\t\tA\tB\tC\t\tD\tE\tF\n";
     for (int i=k;i<=l;++i)
      {   
           if (i<9)
           {cout << "Row ["<<i+1<<"]\t\t";}
           else if (i>=9)
           {cout << "Row ["<<i+1<<"]\t";}
           
           
           for (int j=0;j<=5;++j)
           {
               cout<<seat[i][j]<<"\t";
               if (j==2)
               {cout<<"\t";}
           }
           cout<<endl;
}}

void tictype (int &k, int &l)
{
   char ttype;
   cout<<"Please enter your ticket type \n[F for First Class ; B for Business Class; E for Economy Class] \n[F/B/E] : ";
   cin >> ttype;
   cin.ignore(1000,'\n');            
   cout <<"\nYou have ";
   
   switch (toupper(ttype))
   {
          case 'F' :
               cout<<"first class ticket.";
               k=0;
               l=1;
               break;
          case 'B' :
               cout<<"business class ticket.";
               k=2;
               l=6;
               break;
          case 'E' :
               cout<<"economy class ticket.";
               k=7;
               l=12;
               break;
          default:
               system ("cls");
               cout<<"\nYou have entered an invalid ticket type. Please try again.\n";
               tictype(k,l);
}   }

   
int converty (char y)
{
    int z;
    switch (toupper(y))
    {
         case 'A' :
              z=0;
              break;
         case 'B' :
              z=1;
              break;
         case 'C' :
              z=2;
              break;
         case 'D' :
              z=3;
              break;
         case 'E' :
              z=4;
              break;
         case 'F' :
              z=5;
              break;
         default:
              z=100;
    }                  
    return z;
}

char option ()
{
     int sent2=0;
     char sent;
     do{
                cin.ignore(1000,'\n');
                cout<<"\nDo you wish to continue?\n[y/n] : ";
                cin>>sent;
                if (sent=='Y'||sent=='y')
                {
                    system ("cls");
                    cout<<"You have chosen to continue.\n";
                }
                else if (sent=='N'||sent=='n')
                {
                    cout <<"Thank you for using our service. Have a nice day.\n";            
                }
                else
                {
                    cout<< "\nInvalid selection.";
                    sent2='1';
                }
          }while(sent2=='1');
     return sent;
}
Last edited on
You can remove some braces
eg:
1
2
3
    for (int i=12;i>=0;--i)
       for (int j=5;j>=0;--j)
            seat[i][j]='*';

1
2
if (j==2)
    cout<<"\t";

1
2
3
4
5
6
7
switch (toupper(y))
{
     case 'A' :
          z=0;
          break;
    //...
}

Topic archived. No new replies allowed.