std::string& input function throws bad memory allocation exception

[DragonOsman]

I have to write a program for LearnCPP that uses a for-each loop to search through a fixed-size array for the name input by the user. I'm having to take input in main() because when I tried to do it in a separate function, the program crashed with a non-handled bad memory allocation exception.

Here's the code as it is right now, with the input being taken in in main():

#include <iostream>
#include <string>
#include "keep_window_open.h"

void checkAndPrintResult(const std::string& username, const bool nameFound);

int main()
{
	using namespace std;
	string names[] = { "Alex", "Betty", "Caroline", "Dave", "Emily", "Fred", "Greg", "Holly" };
	cout << "Enter a name: ";
	string username;
	cin >> username;
	cin.ignore(32767, '\n');

	bool nameFound = false;
	for (const auto &name : names)
	{
		if (name == username)
		{
			nameFound = true;
			break;
		}
	}

	checkAndPrintResult(username, nameFound);

	keep_window_open();
	return 0;
}

void checkAndPrintResult(const std::string& username, const bool nameFound)
{
	using namespace std;
	if (nameFound)
	{
		cout << username << " was found.\n";
	}
	else
	{
		cout << username << " was not found.\n";
	}
}

I'm thinking of trying to check for that exception being thrown and dynamically allocating memory for the string& function when and if it is thrown. But I'm not sure if it's possible to use new allocate memory for a string& function and then also have the value be stored in a non-pointer variable so that it can be used in a for-each loop.

[Thomas1965]

What is in this keep_window_open.h ?

[jlb]

I'm having to take input in main() because when I tried to do it in a separate function, the program crashed with a non-handled bad memory allocation exception.

How did you try to take input in a function?

By the way you really should be using std::vector instead of the arrays.

[helios]

Post the code that doesn't work, otherwise how are we supposed to know what went wrong?

[DragonOsman]

This is the code that I had the problem in:

#include <iostream>
#include <string>
#include "keep_window_open.h"

void checkAndPrintResult(const std::string& username, const bool nameFound);
std::string& getStringInput();

int main()
{
	using namespace std;
	string names[] = { "Alex", "Betty", "Caroline", "Dave", "Emily", "Fred", "Greg", "Holly" };
	string username = getStringInput();

	bool nameFound = false;
	for (const auto &name : names)
	{
		if (name == username)
		{
			nameFound = true;
			break;
		}
	}

	checkAndPrintResult(username, nameFound);

	keep_window_open();
	return 0;
}

void checkAndPrintResult(const std::string& username, const bool nameFound)
{
	using namespace std;
	if (nameFound)
	{
		cout << username << " was found.\n";
	}
	else
	{
		cout << username << " was not found.\n";
	}
}

std::string& getStringInput()
{
	using namespace std;
	cout << "Enter a name: ";
	string username;
	cin >> username;
	cin.ignore(32767, '\n');
	return username;
}

@Thomas: keep_window_open.h:

#ifndef KEEP_WINDOW_OPEN_H
#define KEEP_WINDOW_OPEN_H

void keep_window_open();

#endif 

keep_window_open.cpp:

// Function definition for keep_window_open() from the osman_headers.h header file.

#include <iostream>

void keep_window_open()
{
    using namespace std;
    cin.clear();
    cout << "Please enter a character to exit\n";
    char ch;
    cin >> ch;
	cin.ignore();
}

I hope this can help you guys help me fix the problem.

[jlb]

Why are you returning that string by reference instead of by value?

You should just be returning by value. And you really don't need that ignore() call, and be aware that the extraction operator>> stops processing the input when it encounters a whitespace character. I really suggest you use getline() instead.

std::string getStringInput()
{
	std::cout << "Enter a name: ";
	std::string username;
	getline(cin >> std::ws, username);

	return username;
}

[DragonOsman]

I wanted to return the string by reference so that an expensive copy isn't made each time the function is called. I'm kind of used to it.

And since it's only taking in the person's first name rather the full name, do I really need getline()?

One more question: What is std::ws?

[helios]

It's illegal to return references to local objects or variables.

[TheIdeasMan]

Hi,

C++ has copy elision and move semantics, so don't worry about returning a reference in this case.

And get rid of all those using namespace std; , you have them in every function. Just put std:: before each std thing. Google that.

What is std::ws

Well that works exceedingly well when you put it into Google !! :+)

This is really good advice, take notice of it :+) :

jlb wrote:

By the way you really should be using std::vector instead of the arrays.

Good Luck !!

[DragonOsman]

I used a regular fixed array because that's what the specs asked for, otherwise I'd have used a std::vector instead.

And I prefer to use the using directive within functions rather than having to type everything out. Isn't having to type out "std::" in front of those identifiers in function declarations enough?

[TheIdeasMan]

And I prefer to use the using directive within functions rather than having to type everything out.

Did you Google it? You would have seen that it defeats the purpose of having namespaces, by causing naming conflicts. Note that all the expert users do the std:: thing, so I am suggesting you could do it too.

IMO, this is not a personal preference thing, it will cause you problems one day.

The other thing is that I guess some of this code (or the idea) is from Stroustrup's book, is it? If so, then of all the people in the world who I wish wouldn't do that, it's him :+)

I know that this is often not a problem for small amounts of code, but it is still a bad habit IMO.

[Thomas1965]

Note that all the expert users do the std:: thing

The will have their reasons for doing so, but they won't write such trivial programs. I agree that this is not good practice when you work in a company where many people work on the code.

IMO, this is not a personal preference thing

Of course it is, it's not a commandment from God. It's a personal opinion from people who need it in their code.

In Java or .NET it's normal practice to use either.

using System.Collections
or
import java.swing.*;

I have never heard someone complaning about it. Only in this forum seems to be people who want to enforce their opinions on others.

[TheIdeasMan]

@Thomas1965

Just wanted you to know that I always look forward to what you have to say, and that I respect you as a valued member on this forum. However I would like to reply :+)

Of course it is, it's not a commandment from God.

The other half of what I said was: "... ,it will cause you problems one day." I have seen dozens of examples of where this does cause problems.

It's a personal opinion from people who need it in their code.

I look at it as being more of statement of how to do it correctly, and avoid the problems that come from doing this bad practise. And it has nothing to do with the size of the code base, nor how people are working on it.

Maybe Java works differently (I don't know anything about Java), but it is quite easy to have a naming conflict in C++, when one brings in an entire namespace and expose the things that are in the header files. The whole idea of namespaces is to avoid that. Ideally, one should put their own code into it's own namespace/s.

Only in this forum seems to be people who want to enforce their opinions on others.

It's not a matter of enforcing opinion, it's education. Plenty of us here have seen problems that come from this, so that is why is mentioned on this forum.

Anyway, apart from all that :+) Happy Lunar New Year everyone :+D

[Thomas1965]

@TheIdeasMan,

I also value your ideas and opinions. I guess you have much more experience in C++ than I do.
Maybe it is because I had never problems with the namespaces in C++ that I don't understand the problems fully.

The only problem I had with namespaces was in C# where I used a third part library which had a class named HtmlDocument which was also in a different namespace and of course the compiler complained. But it was an easy fix by using a fully qualified name.

When so may people here can't fix a syntax error or create a simple algorithm namespaces are probably the last they need to worry about.

[jlb]

Maybe it is because I had never problems with the namespaces in C++ that I don't understand the problems fully.

Then you'll probably be scratching your head in puzzlement for days when you first encounter a problem caused by name clashes. The warning/error messages tend to be cryptic and hard to understand.

In Java or .NET it's normal practice to use either.

using System.Collections
or
import java.swing.*;

The above doesn't really relate to namespace, it more closely relates to libraries.

[Thomas1965]

In Java or .NET it's normal practice to use either.
using System.Collections
or
import java.swing.*;

The above doesn't really relate to namespace, it more closely relates to libraries.

It does, System.Collections is the namespace, the assembly(library) is System.dll

[helios]

FWIW, I think it's a bad practice in C#, as well. It's just harder to avoid because Sun and Microsoft, in an exercise in pointlessness, turned the namespace dial up to 11 and then back down to -300. Boost also has some namespace silliness, but C++ at least has scoped using.

Some time ago at work we came across the following issue: someone had tagged a function with the debug-only attribute and (IIRC) also surrounded the corresponding using with an #if (because it wasn't used anywhere else, I guess). This caused the release build to fail. It turned out that the type in question existed by that name somewhere in the CLR and also in our project, so hiding the using caused the compiler to pick the type in the project.
Luckily, the types had incompatible interfaces. You can imagine what fun it would have been to debug that.