Soda Machine Rough Draft. Not complete. Need efficiency/test help

[PatricBernard]

Here is the next bit of code that I am writing to practice if else statements, and also using functions outside of the main function. I also wrote this code without using mainspace std;.

I am still working on the tester loop so that the user cannot enter in an invalid entry.

Does anyone have any tips on how I could enter that as a seperate function? I could in theory transfer the value of int selection to a different function that tests the value and repeats the original text if the invalid entry is given, but what would be the most efficient way to do it?

Again, my goals are to get the basics down to a T before I start getting more advanced. I have already been learning a lot from these forums, so any constructive criticism is greatly appreciated.

#include <iostream>
int select(int x) // Function is to handle the users selection. Determines which drink to deposit.
{
           if (x == 1)
           {
                 std::cout<<"Here is your Coke!  *Clunk Clunk*" << std::endl;
                 }
              else if (x==2)
              {
                   std::cout<<"Here is your Sprite! *Clunk Clunk*" << std::endl;
                   }
              else if (x==3)
              {
                   std::cout<<"Here is your Dr. Pepper! *Clunk Clunk*" << std::endl;
                   }
                   else if (x==4)
                   {
                        std::cout<<"Here is your Mt. Dew! *Clunk Clunk*"<< std::endl;
                        }
                        else if (x==5)
                        {
                             std::cout<<"Here is your Water! *Clunk Clunk*"<< std::endl;
                             }
                             else
                             {
                                 std::cout<<"Please enter in a given value 1-5 for your selection."<<std::endl;
                                 }
              return 0;
              }
int main()
{
    int selection;
    std::cout<<"Hello! Welcome to the Soda Machine!"<<std::endl; 
    std::cout<<"Please make a selection!"<<std::endl; 
    std::cout<<"1: Coke"<<std::endl; 
    std::cout<<"2: Sprite"<<std::endl;
    std::cout<<"3: Dr. Pepper"<<std::endl; 
    std::cout<<"4: Mt. Dew"<<std::endl; 
    std::cout<<"5: Water"<<std::endl; 
    std::cin>>selection;
    
    if(selection >=1 && selection <=5) // loop ensurs the user cannot enter in an invalid figure. Not full proof yet.
    {
    select(selection);
    }
    std::cin>>selection; // extra ci>>selection is to keep program from closing. For personal use.
    
    return 0;
}

[IceThatJaw]

I notice a pattern with your if/else statements. They are all comparing an int to an int.

This screams for a switch statement. You don't have to use the switch/case, but you should. Modern compilers are tricky but most of the time a switch statement will be slightly faster than if/else statements. It's also cleaner code.

As for the valid entry loop, you could set a flag on the default case and use a do while loop to check it the case is invalid.

bool Select(int x) // Function is to handle the users selection. Determines which drink to deposit.
{
		bool valid = true;
		switch(x)
		{
        	case 1 : std::cout<<"Here is your Coke!  *Clunk Clunk*"<< std::endl;
        		break;    
                case 2 : std::cout<<"Here is your Sprite! *Clunk Clunk*"<< std::endl;
           		break;    
                case 3 : std::cout<<"Here is your Dr. Pepper! *Clunk Clunk*"<< std::endl;
           		break;
                case 4 : std::cout<<"Here is your Mt. Dew! *Clunk Clunk*"<< std::endl;
           		break;
	        case 5 : std::cout<<"Here is your Water! *Clunk Clunk*"<< std::endl;
    			break;                         
                default : 
			{			
		        std::cout<<"Please enter in a given value 1-5 for your selection."<< std::endl;
    			valid = false;
   		 	}
		} 
		return valid;	
}

int main()
{
    int selection;
 	
	do
	{
	   	std::cout<<"Hello! Welcome to the Soda Machine!"<<std::endl; 
		std::cout<<"Please make a selection!"<<std::endl; 
		std::cout<<"1: Coke"<<std::endl; 
		std::cout<<"2: Sprite"<<std::endl;
		std::cout<<"3: Dr. Pepper"<<std::endl; 
		std::cout<<"4: Mt. Dew"<<std::endl; 
		std::cout<<"5: Water"<<std::endl; 
		std::cin>>selection;
	}
	while (!Select(selection)); // loop ensurs the user cannot enter in an invalid figure. 
    
    return 0;
}

[Zaita]

Hi!

Some tips for you.

1. From a style point of view, do not indent each part of the if-else in further than the parent. Functionaly they are all at the same level and should not be indented.
e.g

 if (x == 1) 
  cout << "x is 1" << endl;
 else if (x == 2)
  cout << "x is 2" << endl;
 else if (x == 3)
  cout << "and so on" << endl;

2. To ensure the input is valid, do not use cin >> selection. Instead use std::getline()

string input = "";
std::getline(cin, input);

// convert input to unsigned 

3. This doesn't really need an if-else, because the user is entering a number you could use a switch statement.

4. There is no need to have an int return type on select when you're not going to use it. Use void select(const int &select);

5. To save on typing std::cout and std::cin use. *Note: don't use "using namespace std;" because this includes the whole namespace when it's not necessary.

#include <iostream>

using std::cout;
using std::cin;

int main() {
 cout << "Hello World!" << endl;
 return 0;
}

[PatricBernard]

Can you explain how exactly the break statement works? I remember learning about it in school, but was never required to use it, so I just never picked it up. It definitely looks a lot simpler and a lot faster to me though.

Also, can you please explain this line?

while (!Select(selection));

I'm not 100% sure on how that loop runs. Maybe I'm just having an over thinking moment here.

And thank you for the help with the std::cout/cin short cut. That would definitely be a lot easier. Also, getline would be more secure when it comes to valid inputs? Why is that?

And thank you for the style suggestion. I was never sure on how I should write, but that looks a lot better to me haha.

[Lowest0ne]

Switch statements look like this:

switch (value)
{
  case 1:  /* statements */ /* optional break; */
  case 2:
  /* optional default: */
}

Upon calculating the value, the program jumps to that case. For example, if value was 2, then it would go directly to case 2. It would never wonder if 2 == 1, so that is how switches are faster.

The optional [cpde]break;[/code] is inserted into the case statements if you do not want the case to flow down. Without a break in case 1, the statements in case 2 would happen.

The optional default is what happens when value calculates to something that is not a case. Good for error reporting.

Getline is more secure because you can't "break" the cin object as easily. >> expects the type that it is extracting. If you had asked for an int, but gave it a letter, it would break. The drawback of getline is that now you have a string that needs to be converted to an int if you want to use it as an int.

I prefer to check if the cin object was broken. There is a better way to make your code, and this includes no switch or if else conditions.

Make an array (or better, vector), of soda names, and then insure that the user enters a number that is within this range:

//KqQkK5
#include <iostream>
#include <vector>
#include <string>

double getNum(void);
int getInt(void);
int getIntInRange(int low, int high);

int main(void)
{
  // This requires c++ 11
  std::vector<std::string> drinks = 
  { "Coke", "Pepsi", "Root Beer", "Sprite", "Fanta" };

  // If you do not have c++ 11, you can do
  //const char *d[] = 
  //{ "Coke", "Pepsi", "Root Beer", "Sprite", "Fanta" };
  //std::vector<std::string> drinks(sizeof(d) / sizeof(const char*));
  // for (unsigned int i = 0; i < drinks.size(); i++)
  //  drinks[i] = (std::string(d[i]));

  
  std::cout << "Welcome to the Soda Machine\n"
        << "Please make your selection\n";
  for (unsigned int i = 0; i < drinks.size(); i++)
    std::cout << i + 1 << ") " << drinks[i] << std::endl;
  std::cout << "\n: ";  
  
  int n = getIntInRange(1, drinks.size()) - 1;
  std::cout << "You picked: " << drinks[n] << '\n';
  return 0;
 }
 
double getNum(void){
  double n;
  while (!(std::cin >> n))
  {
    std::cin.clear();
    std::cin.ignore(80, '\n');
    std::cout << "Numbers only please, try again: ";
  }
  std::cin.ignore(80, '\n');
  return n;
}
int getInt(void){
  double n = getNum();
  while (n != static_cast<int>(n))
  {
    std::cout << "Integers only please, try again: ";
    n = getNum();  
  }
  return static_cast<int>(n);
}
int getIntInRange(int low, int high){
  int n = getInt();
  while (n < low || n > high)
  {
    std::cout << "Number must be in range (" <<low<<", "<<high <<")\n"
          << "Try again: ";
    n = getInt();  
  }
  return n;
}

With that code, all you have to do is change line 13 to be the drinks that you want.

[PatricBernard]

That code seems really confusing, but I think that's because I don't know what's going on haha. I'll need to go through and analyze it carefully

[IceThatJaw]

I can see why this snippet would be confusing.

double getNum(void){
  double n;
  while (!(std::cin >> n))
  {
    std::cin.clear();
    std::cin.ignore(80, '\n');
    std::cout << "Numbers only please, try again: ";
  }
  std::cin.ignore(80, '\n');
  return n;
}

What is happening is the code is checking to see if the std input stream failed. If you try to read in a datatype that doesn't match up (excluding strings and chars), the stream will set a number of flags and stop. This code is checking one of the flags and if it is set, it is clearing out stream and resetting it to be used again.

The cin.clear() method resets the flag and the cin.ignore() method clears out and remaining chars in the stream by ignoring them. You typically ignore 80 or 256 chars or till the newline char is found.

Honestly, you can accomplish the task without all the extra methods if you just read in a single character. Since you aren't doing any Math with the selection, there really isn't any reason to use an int.

bool Selected(char x) // Function is to handle the users selection. Determines which drink to deposit.
{
		bool valid = true;
		switch(x)
		{
        	case '1' : std::cout<<"Here is your Coke!  *Clunk Clunk*"<< std::endl;
        		break;    
            case '2' : std::cout<<"Here is your Sprite! *Clunk Clunk*"<< std::endl;
           		break;    
            case '3' : std::cout<<"Here is your Dr. Pepper! *Clunk Clunk*"<< std::endl;
           		break;
            case '4' : std::cout<<"Here is your Mt. Dew! *Clunk Clunk*"<< std::endl;
           		break;
	        case '5' : std::cout<<"Here is your Water! *Clunk Clunk*"<< std::endl;
    			break;                         
            default : 
			{			
		        std::cout<<"Please enter in a given value 1-5 for your selection."<< std::endl;
    			valid = false;
   		 	}
		} 
		return valid;	
}

int main()
{
    char selection;
 	
	do
	{
	   	std::cout<<"Hello! Welcome to the Soda Machine!"<<std::endl; 
		std::cout<<"Please make a selection!"<<std::endl; 
		std::cout<<"1: Coke"<<std::endl; 
		std::cout<<"2: Sprite"<<std::endl;
		std::cout<<"3: Dr. Pepper"<<std::endl; 
		std::cout<<"4: Mt. Dew"<<std::endl; 
		std::cout<<"5: Water"<<std::endl; 
		std::cin>>selection;
	}
	while (!Selected(selection)); // loop ensurs the user cannot enter in an invalid figure. 
    
    return 0;
}

[Lowest0ne]

Honestly, you can accomplish the task without all the extra methods if you just read in a single character. Since you aren't doing any Math with the selection, there really isn't any reason to use an int.

If I change line 14 (and perhaps 24 too) I can have an entirely different menu. You would have to write a new program. I can copy/paste those three functions into any program and never have to worry about getting numbers again.

Your program has big troubles when there are more than 10 options, I just write another word at line 14 and there it is.