HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Reading file in C++ until string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readingfilestringuntil

Problem

I need to read HTML files and ignore the input until reaching the following tag `` tag.

Below is the extracted line of the file of interest.

Phil Jankins
                                


Below is my approach

#include 
#include 
#include 
#include 
#include 
using namespace std;
int main()
{
string temp;
string delimet = "member - modal";

ifstream file;
file.open("in.txt");

getline(file, temp);
cout << temp;
while (!file.eof())
{
    if (temp.find(delimet))
        cout << temp;
    getline(file, temp);
}
file.close();

}

Solution

Unnecessary #includes:

You don't need ` and for this program. Those can be removed.

Avoid
using namespace:

using namespace in a source file for such a tiny program is fine, but don't make it a habit. Prefer to qualify the names with the namespace prefix (std::) when practical. Read a longer discussion here.

Open the stream on declaration and let the destructor close it:

Instead of explicitly calling
open() in your file, use the parameterized constructor to make your code more concise and direct:

std::ifstream file("in.txt");


Also, no need to explicitly close at the end of
main(). The destructor will close the file automatically when the scope of declaration is exited.

Be sure to always check that the file was opened successfully! If it fails, you should at least print a message to let the user know about the problem.

Use the return value of
getline() instead of eof():

The canonical line reading loop in C++ is:

while (std::getline(file, line)) 
{
}


That can simplify you program quite significantly. You can still test specific error states with the methods
bad(), fail() and eof(). See the documentation for std::ifstream.

Incorrect use of
string::find():

You have misinterpreted the return value of
std::string::find(). This method returns the position of the first character of the first match. If no matches were found, the function returns std::string::npos.

Which means that testing it as in
if (temp.find(delimet)) will probably not work as you intended, since the return value is not a boolean.

The correct usage is:

if (str.find(substring) != std::string::npos) { /* found the substring */ }


Also note that
"member - modal" is not the same as "member-modal". find() will look for an exact match, including spaces, so your implementation will only find something in the input file if you replace the search string to match the contents of the file down to each space. A more flexible implementation could be achieved using C++ regular expressions.

Be more thoughtful about variable naming:

Names line
temp and delimet convey little meaning.

temp should at the very least be named line, as that is what the variable holds (a line from the file).

delimet sounds like a misspelled "delimiter", so delimiter would be better. Other options include needle, for searching a needle in a haystack, target, for the target of the search, wanted_string, and the list goes on...

Properly indent
main():

Code inside
main()` is not properly indented to convey nesting into the function. Proper indentation is very important for code readability, so that must be fixed.

Summing up:

This is my suggest implementation, based on the original code and applying the above changes:

#include 
#include 
#include 

int main()
{
    // Alway check after opening to make sure the
    // file existed and was opened successfully.
    //
    std::ifstream file("in.txt");
    if (!file.is_open())
    {
        std::cerr Phil Jankins
            // once
        }
    }
}

Code Snippets

std::ifstream file("in.txt");
while (std::getline(file, line)) 
{
}
if (str.find(substring) != std::string::npos) { /* found the substring */ }
#include <fstream>
#include <iostream>
#include <string>

int main()
{
    // Alway check after opening to make sure the
    // file existed and was opened successfully.
    //
    std::ifstream file("in.txt");
    if (!file.is_open())
    {
        std::cerr << "Failed to open file!\n";
        return -1;
    }

    // Since this is a read-only variable,
    // also mark is as `const` to make intentions
    // clear to the readers.
    //
    const std::string needle = "member-modal";

    // Main reading loop:
    //
    std::string haystack;
    while (std::getline(file, haystack))
    {
        if (haystack.find(needle) != std::string::npos)
        {
            std::cout << haystack << "\n";
            // Prints 
            //  <a class="member-modal" href="https://website.net/users/membercard/890520">Phil Jankins</a>
            // once
        }
    }
}

Context

StackExchange Code Review Q#83063, answer score: 6

Revisions (0)

No revisions yet.