OpenFile function in C++ I am in the process of learning basic C++ and one of the practice questions I came across was to write a function to open a file in C++.

I did understand how to use the streams but I think I went overboard a little bit by making sure that it works. I would appreciate any feedback on my code. The objective was also to keep prompting the user to open a file if it doesn't work in the first go.

Code:

void OpenFile(std::ifstream&amp; inputFileStream) {
    while(true) {
        inputFileStream.clear();
        std::stringstream fileNameStream;
        std::string fileName;
        std::cout &lt;&lt; "Enter the file name to open: ";
        if(!getline(std::cin, fileName)) {
            std::cout &lt;&lt; "Please enter a valid name." &lt;&lt; std::endl;
        } else {
            fileNameStream &lt;&lt; fileName;
            std::string fileToOpen;
            std::string remaining;
            if(fileNameStream &gt;&gt; fileToOpen) {
                if(fileNameStream &gt;&gt; remaining) {
                    std::cout &lt;&lt; "Unexpected character: " &lt;&lt; remaining &lt;&lt; std::endl;
                } else {
                    inputFileStream.open(fileToOpen.c_str());
                    if (!inputFileStream.is_open()) {
                        std::cout &lt;&lt; "File could not be opened. Please try again.";
                        std::cout &lt;&lt; std::endl;
                    } else {
                        break;
                    }
                }
            }
        }
    }
}
Prefer returning a result instead of passing it via a reference

The result of your function is a std::ifstream object, so make that explicit:

std::ifstream OpenFile() {
  while(true) {
    // read filename

    std::ifstream file(fileName);

    if (file.is_open())
      return file;
    else
      // handle error
  }
}


The advantage is that instead of having to write:

std::ifstream file;
OpenFile(file);


You can write:

std::ifstream file = OpenFile();


Or even:

auto file = OpenFile();


Avoid using std::stringstream if it's not necessary

It might seem convenient, but it's rather inefficient, and often there are other ways to accomplish what you want. It seems from your code that you want to forbid filenames with whitespace characters in them. Consider using std::find_first_of() to check if there are certain characters in a string.

Apart from that, many filesystems, including those on mainstream operating systems such as Linux, Windows and macOS, allow spaces in their filenames. And surely, when you call open(), the operating system will return an error if you pass it an invalid filename, so you don't have to do this error checking yourself.

Avoid casting std::string to C-strings

Since C++11 you can pass a std::string directly to a std::ifstream's constructor or to its open() function.

Use '\n' instead of std::endl

Avoid using std::endl. It is equivalent to '\n' plus a call to flush(). The flush is usually unnecessary and might hurt performance. So for example, instead of:

std::cout &lt;&lt; "File could not be opened. Please try again.";
std::cout &lt;&lt; std::endl;


Write:

std::cout &lt;&lt; "File could not be opened. Please try again.\n";


Pass along the error message from the operating system

There are many reasons why opening a file might fail. It will benefit the user to learn the exact reason why it failed. For example, it could be that the file doesn't exist, or that it's permissions don't allow it being opened by the user. To do this, call strerror(errno), like so:

std::cout &lt;&lt; "File could not be opened: " &lt;&lt; strerror(errno) &lt;&lt; "\nPlease try again.\n";


Add a way for the user to cancel opening a file

The function loops forever until a file has succesfully been opened. What if the user decides that it doesn't want any file to be opened at this time, but wants the application to cancel this operation but do something else? If you don't allow a way out of this function, the only option available to the user is closing the application and restarting it.

Consider making the function more generic

While you shouldn't add bells and whistles to a function like this if you are not going to use them, you might want to consider whether you want to make this function a bit more widely applicable. For example, you could have it open a file either for reading, writing or both, or have a way to give a custom prompt. For example:

std::fstream OpenFile(ios_base::openmode mode = ios_base::in, const std::string &amp;prompt = "Enter a file name: ") {
  while(true) {
    std::cout &lt;&lt; prompt;
    // read filename

    std::fstream file(filename, mode);

    if (file.is_open())
      return file;
    else
      // handle error
  }
}