problems with getline() and cin.ignore()

[navilgameboy]

So, i got a problem where it would repeat "write something: " twice. Which i fount out was triggered by the getline(cin, user_input) or getline(cin, current_user_profile) commands and that i should use the cin.ignore command. But now,if i write something like for ex. "write something: HOW ARE YOU"
it says "you wrote: ow are you"
Could i please get some help?
Thanks in advance.

#include <iostream>
#include <stdio.h>
#include <algorithm>
#include <string>
#include <fstream>
#include <conio.h>
#include <math.h>
#include <Windows.h>
#include <limits>
using namespace std;

char user_input_inchar[256];

string user_input;
string current_user_profile, current_user_password;
string username, password;
string newprofile;
string makenewusername, makenewpassword;

int user_birth_year;
int currentyear = 2019;
int user_age;
int current_month;
int user_birth_month;
int user_bmonth;


void TalkToAi() { //VOID TALKTOAI
	while (true) {
		cout << "write something: ";
		cin.ignore();
		getline(cin, user_input);
		transform(user_input.begin(), user_input.end(), user_input.begin(), ::tolower); //TRANSLATES ALL UPPERCASE LETTERS TO LOWER CASE SO THE SYSTEM CAN UNDERSTAND!
					cout << "you wrote: " << user_input << "\n";
					//IF LIBRARY!!!
						if (user_input == "what's my age?" || user_input == "count my age" || user_input == "whats my age?" || user_input == "how old am i?") {
							cout << "when were you born?\n" << "born: ";
							cin >> user_birth_year;
								user_age = currentyear - user_birth_year;
									cout << "what month is it? (in numbers)\n" << "month: ";
									cin >> current_month; 
									cout << "what month were you born? (in numbers)\n" << "month: ";
										cin >> user_birth_month;
											if (current_month <= user_birth_month) {
												user_age = user_age - 1;
											}
												user_bmonth = current_month - user_birth_month;
													if (user_birth_month == 12) {
														user_bmonth = current_month;
													}
														cout << "you are " << user_age << " years old and " << user_bmonth << " months old\n";

		}
						if (user_input == "open google" || user_input == "launch google") {
							ShellExecute(0, 0, L"http://www.google.com", 0, 0, SW_SHOW);
						}
	}
}

	void StartUp() { //VOID UPONSTARTUP (WHAT TO DO, ALSO READS DIFFRENT PROFILES AND PASSWORDS FOR LOGIN)
	cout << "what profile should i load?" << "\n" << "profile name: ";
	getline(cin, current_user_profile);

		fstream Myfile; //OPEN FILE TO STORE USERNAMES
		Myfile.open("Usernames_and_passwords.txt", fstream::in | fstream::out | fstream::app);

			if (Myfile.is_open()) {
				while (getline (Myfile, username) ) { //CHECKS IF USERNAME EXISTS
					if (username == current_user_profile && password == current_user_password) {
					cout << "\n" << "Hello, " << username << "\n";
					return;
				}
				}

					cout << "wrong username or username unfortunately not found.\n"; //IF IT DOESN'T, CREATE NEW USERNAME
					cout << "shall i create a new profile? Yes/No: ";
						cin >> newprofile;
							if (newprofile == "Yes" || newprofile == "yes") {
								cout << "new profile username: ";
								cin >> makenewusername;
									Myfile.clear();
									Myfile.seekg(0, ios::beg);
									Myfile << makenewusername << endl;
										if (newprofile == "no" || newprofile == "No") {
											return;
										}

							}
			} 
}



int main() {
	StartUp(); //CALLS VOID STARTUP
	TalkToAi(); //CALLS VOID TALKTOAI
}

[Peter87]

cin.ignore(); belongs after you have finished reading a row using cin >> ... rather than before getline(cin, ...

[dhayden]

Duplicate post: http://www.cplusplus.com/forum/beginner/252219/

[Handy Andy]

@dhayden.

Not an exact duplicate. this one has the misuse of cin.ignore();

Hello navilgameboy,

Testing your program I found many problems.

In the "#include" statements:

"stdio.h" this is not a standard C++ header file, but a C header file. FYI the proper header file would be "cstdio". Anyhow this is covered through "iostream" which eventually includes "cstdio" along with "cmath" and others.

"conio.h" again this is not a standard C++ header file.
salem c once wrote:

> #include<conio.h>
Obsolete since 1990, when the world stopped using DOS as a primary operating system.

Also not everyone has this header file available as some of the IDEs and compilers no longer include this. In the end you use nothing from this header file in your program.

"math.h" should be"cmath", but like "conio.h" I see nothing in this program that would need this file.

This is the tricky one:

#include <Windows.h>
#include <limits>  

In "minwindefs.h" it defines max as: #define max(a,b) (((a) > (b)) ? (a) : (b)) but in "limits" the function is completely different. About two years ago I was told if you use "Windows.h" and "limits" in the same file you will need to use:

#include <Windows.h>
#undef max  // <--- From the "Windows.h" definition.
#include <limits>  

Then when you use: std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // <--- Requires header file <limits>. there should not be any problem with "max".

For now I will just say this: using namespace std; // <--- Best not to use.

This line: char user_input_inchar[256]; would be better defined as a "std::string", but that is irrelevant as it is never used in the program.

Lines 14 - 18 are global variables that are better put in the functions that use them or passed to a function if needed. If a variable passed needs its changes back in the calling function then pass by reference.

As a global with file scope any function can change the value of the global variables and it can be difficult to find where a problem is.

In the function "StatrUp()":

You open "Myfile", should be (myFile) as a variable like this should start with a lower case letter, as an "fstream". The next line opens the stream as (in, out and append) and this is the start of the problem.

The following if statement will always be true because if the file name does not exist opening the stream for output will create the file name which makes the if statement true, so you could be reading an empty file and not realize it.

It would work better to only open the file for input, close it when you are done and then open the file for output and append when it is time.

This way you could write your code as:

fstream myFile; //OPEN FILE TO STORE USERNAMES
myFile.open("Usernames_and_passwords.txt", fstream::in);

if (!myFile)
{
	std::cout << "\n  File " << std::quoted("Usernames_and_passwords.txt") << " did not open" << std::endl;
	/*return 1;*/ exit(1);  // <--- Use exit if not in "main".
}

"std::quoted()" comes from the header file "iomanip". I have found that #include <iostream> is best followed by #include <iomanip> . Just my opinion.

You may also need some kind of pause before the "exit(1)" to give you time to read the message.

If the stream does not open you need to leave the program and fix the problem before you continue with the rest of the program.

This will eliminate the need for your if statement that does not work. Also it is not the best form to put the code in an if or else block.

The while loop is OK for reading the file, but if the loop ends and you continue you need to close the input file before you open it for output.

The variable "newprofile" should be defined as a "char" not a "std::string" and the prompt needs to ask for "Y" or "N". By including the header file "cctype" you can use the functions "std::tolower()" or "std::toupper()" then your if statement would be: if (std::tolower(newprofile) == 'y') then you would not need all the ||s to check for every possible answer.

In the if statement you have:

cout << "new profile username: ";
cin >> makenewusername;
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.  

This is your last use of std::cin >> and where you need to follow it with the "ignore". Placing it before a "std::getline" can work, but you need to be careful how it is used.

Either you had a problem with the "max()" function in the above ignore or yo do not understand the ignore function. "ignore" takes two parameters the first is a number, usually a large number. The std::numeric_limits<std::streamsize>::max() returns the largest possible value for what you are using, i.e., operating system, IDE and compiler. I have also seen 1000 used quite often. The second parameter is the delimiter, usually the "\n", but could be anything else as you need it. When you use cin.ignore(); you are using the default parameters of "1" and "\n".

The "ignore" will ignore the number of characters of the first parameter or the delimiter whichever comes first. So cin.ignore(); will ignore one character and move on. There are a few occasions where this will work, but not every time.

The two lines:

myFile.clear();
myFile.seekg(0, ios::beg);

Are pretty much pointless at this point. The "clear" resets the state bits so you can use the file stream again, but if you open the file for just input and close it when you are done before you open the file stream for output this code is irrelevant to the program. And since you never read the input file a second time resetting the file point to the beginning has no use.

Now for the "TalkToAi" function:

I have nor worked on this part fully yet, but this is what I can see for now.

The while condition is based on "true" and therefore it is an endless loop with no way out. Like the Energizer Bunny it just keeps going and going.

The "cout statement "write something: " is misleading. It makes me think I can write anything, but the if statement is looking for something else and I have no idea what based on the prompt.

The "ignore" statement before the "std::getline()" will work the first time through the while loop, but is becomes a problem if you do not enter the first if block. But if you follow the last "cin.ignore" in the other function it is not needed here.

Should you enter the first if block the last std::cin >> needs to be followed by the ignore statement.

Your prompt: cout << "when were you born?\n" << "Born: "; makes me feel that I should enter a full date when all you want is the year. This would work better: cout << "When were you born?\n" << "Birth year: "; it is a better idea of what you are expecting. If you want to use a whole date you can write the "cin" as: std::cin >> month >> junk << day <<junk << year; with "junk" defined as a "char".

I have not fully tested the rest of the if statement, but the line: cin >> user_birth_month; needs to be followed with: std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // <--- Requires header file <limits>.

In the second if statement you have: ShellExecute(0, 0, L"http://www.google.com", 0, 0, SW_SHOW);. The "L" in front of the quoted string is not needed. With my IDE and compiler it produces an error. When I removed the "L" it worked fine.

Hope that helps

Andy

[Handy Andy]

Hello navilgameboy,

Some tips that did not fit the last post:

Watch your indenting. What is posted here the indenting tends to be exaggerated making what you did look worse.

As a quick example your code:

int main() {
	StartUp(); //CALLS VOID STARTUP
	TalkToAi(); //CALLS VOID TALKTOAI
}

Looks better as:

int main()
{
	StartUp(); //CALLS VOID STARTUP
	TalkToAi(); //CALLS VOID TALKTOAI
}  // End main. 

When the {}s line up in the same column it makes it easier to read and easier to find a missing closing }. Also if you put a comment on the closing } brace it helps as does other comments in the program.

When it comes to naming things a simple variable should start with a lower case letter. I would say that "camelCase" is the most often used method of combining two or more names. "camelCase" meaning that the second or more word(s) is/are capitalized. The second option is to use the underscore character to denote a space between words. With whatever method you use stay with and be consistent in its use. Mixing can be confusing.

When it comes to a class or a struct they should start with a capital letter.

A variable defined with "const" or "constexpr" should be in all caps. To me this helps to remind me that it is defined as a constant.

Lastly use some blank lines. The compiler does not care about white space or blank lines, but it does make a big difference to the reader.

Here is a good example of two things:

		cout << "wrong username or username unfortunately not found.\n"; //IF IT DOESN'T, CREATE NEW USERNAME
		cout << "shall i create a new profile? Yes/No: ";
		cin >> newprofile;
		if (newprofile == "Yes" || newprofile == "yes") {
			cout << "new profile username: ";
			cin >> makenewusername;
			Myfile.clear();
			Myfile.seekg(0, ios::beg);
			Myfile << makenewusername << endl;
			if (newprofile == "no" || newprofile == "No") {
				return;
			}
		}
	}
}

First when all the lines of code run together it is hard to read. Second you have four lines of closing braces. For lines 12 and 13 you can see where the opening brace is, but lines 14 and 15 the opening braces are not on the screen, so it makes it harder to find where the opening brace is.

This makes the code a little easier to read:

		cout << "wrong username or username unfortunately not found.\n"; //IF IT DOESN'T, CREATE NEW USERNAME
		cout << "shall i create a new profile? Yes/No: ";
		cin >> newprofile;

		if (newprofile == "Yes" || newprofile == "yes")
		{
			cout << "new profile username: ";
			cin >> makenewusername;
			std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.

			myFile.clear();
			myFile.seekg(0, ios::beg);
			myFile << makenewusername << endl;

			if (newprofile == "no" || newprofile == "No")
			{
				return;
			}
		}
	//} // <--- No longer needed. End of "if (myFile.is_open())"
}

Sorry I just put a comment on the if statement and closing } and did not unindent the lines of code between them.

I hope you find this useful. Any questions let me know.

Hope that helps,

Andy

[Peter87]

This last post by Handy Andy contains many of his own opinions. While I and many others do agree with many of these things other people might have other opinions. Just saying.

"stdio.h" this is not a standard C++ header file

This is not entirely true. "stdio.h" will include everything that "cstdio" does. The difference is that the names are added to the global namespace rather than the std namespace. The .h headers are listed as deprecated which means they are not recommended and might be removed in the future (even though in this case it's probably not going to happen anytime soon).