How to return a string line by line in a function?

Question

I am reading a text file which contains integers separated by a new line. Like this.

5006179359870233335
13649319959095080120
17557656355642819359
15239379993672357891
3900144417965865322
12715826487550005702

From this file, I want to access each integer in a loop and compare it with another, in order to match those two. In function File_read() I can print the integers. But what I want is to get it integer by integer outside the function. For example in main method, if there is a integer called x, I want to check whether x equals one of the integers in my text file.

string File_read() {
    std::ifstream my_file("H:\\Sanduni_projects\\testing\\test.txt", 
    std::ifstream::binary);
    if (my_file) {
        string line;

        for (int i = 0; i < 25; i++){
            getline(my_file, line);
            //cout << line << endl;
            return line;
        }

        if (my_file)
            std::cout << "all characters read successfully."<<endl;

        my_file.close();
    }
    return 0;
}

when I was calling this function, only the first integer of the text file returns. But when I change my code replacing return line; with //cout << line << endl; , which I have commented, I can read all integers — , Jun 01 '17 at 07:03
Please [edit] your question instead of amending it with comments. — n. m. could be an AI, Jun 01 '17 at 07:09

Felix Glas · Answer 1 · 2017-06-01T08:08:54.247

1

Never return unconditionally inside a loop.

You are returning unconditionally from inside the loop. This causes the caller to exit the loop and return from the function during the first iteration.

for (int i = 0; i < 25; i++){
    getline(my_file, line);
    return line; // <-- Return from function (rest of iterations unreachable). Bad.
}

No need to reinvent stuff

Use the standard library to read the numbers, e.g., into a container std::vector.

std::vector<unsigned long long> v{std::istream_iterator<unsigned long long>{my_file},
                                  std::istream_iterator<unsigned long long>{}};

Notice the value type of unsigned long long that is needed to fit the large numbers (you're pushing ~64 bits here).

Find a match

Use, e.g., std::find to find a possible match among the parsed numbers.

auto key = 15239379993672357891ull;

if (auto it = std::find(std::begin(v), std::end(v), key); it != std::end(v)) {
    std::cout << "Key found at line " << std::distance(std::begin(v), it) + 1 << std::endl;
}

Here, I'm using a C++1z if(init; condition) statement to limit the scope of the iterator it to inside the if statement. It's optional of course.

Live example

edited Jun 01 '17 at 08:08

answered Jun 01 '17 at 07:47

Felix Glas

15,065
7
53
82

Why do you `+ 1` to the distance? – Jonas Jun 01 '17 at 08:04
Maybe you should add a note on the most vexing parse problem in case OP cannot use aggregate initialization when populating the vector. – Jonas Jun 01 '17 at 08:08
@Jonas Because line number counts are IMHO most intuitively described as starting on line 1, not line zero. – Felix Glas Jun 01 '17 at 08:10
@Jonas Hmm, most vexing parse seems to me like a problem of the past, when curly-brace initialization, sometimes called uniform initialization, is available (aggregate initialization is something else). Your comment is welcome though. – Felix Glas Jun 01 '17 at 08:17
The "Never return unconditionally inside a loop" is somewhat strong, because its the desired semantic when searching. Although you could replace the search with find(_if) – Caleth Jun 01 '17 at 08:33
@Caleth "_its the desired semantic when searching_" - then it's not unconditional, is it? `std::find` has a condition to only return _when a match is found_, or the end is reached. – Felix Glas Jun 01 '17 at 08:44

Jonas · Accepted Answer · 2017-06-01T08:04:42.063

0

You are, currently, just returning the first number (as a std::string and not a number). If you remove the return statement in your loop you can, of course, print each of them. Here is a slightly modified version of your File_read function that will return a std::vector<unsigned long long> that contains all the numbers. Then you can use this vector in, e.g., your main function to do your processing.

std::vector<unsigned long long> File_read()
{
    std::vector<unsigned long long> numbers;
    std::ifstream my_file("H:\\Sanduni_projects\\testing\\test.txt"); // Text files are not 'binany', i.e., removed std::ifstream::binary
    if (my_file)
    {
        std::string line;

        for (int i = 0; i < 25; i++)
        {
            std::getline(my_file, line);
            numbers.push_back(std::stoull(line));
        }

        if (my_file)
        {
            std::cout << "all characters read successfully." << std::endl;
        }

//        my_file.close(); // Do not do this manually
    }
    return numbers;
}

Usage example:

int main()
{
    unsigned long long x = /* some number */;

    // Read all the numbers
    std::vector<unsigned long long> vl = File_read();

    // Run through all the numbers
    for (unsigned long long y : vl)
    {
        // Check if any of the numbers are equal to x
        if (x == y)
        {
            // There is a match...
            // Do stuff
        }
    }
}

Update

The numbers cannot be held by in a long, however unsigned long long is sufficient.

edited Jun 01 '17 at 08:04

answered Jun 01 '17 at 07:08

Jonas

6,915
8
35
53

I have changed the code according to yours. I builds without errors. But while running it breaks giving this Unhandled exception at 0x000007FEFD0BB3DD in AdDetectionMPEG4.exe: Microsoft C++ exception: std::out_of_range at memory location 0x00000000002BFA90. – Jun 01 '17 at 07:48
`long` is not enough to fit those numbers. – Felix Glas Jun 01 '17 at 07:48
@SanduniWickramasinghe As @Snps has pointed out I forgot to make the type `unsigned`, so instead of `long` you should use `unsigned long long`. I have updated the answer. – Jonas Jun 01 '17 at 07:56
@Jonas `unsigned long` is only guaranteed to fit 32 bits. This will still not be enough. `unsigned long long` (at least 64 bits guaranteed) is needed. – Felix Glas Jun 01 '17 at 08:00
@Snps You are, strictly speaking right, but on most 64 bit architectures `long` is also 8 bytes. I Have updated the answer. – Jonas Jun 01 '17 at 08:02
1

problem still exists although I made it unsigned long – Jun 01 '17 at 08:03
1

@SanduniWickramasinghe Also change `std::stol` to `std::stoull`, in `numbers.push_back(std::stoull(line));` – Jonas Jun 01 '17 at 08:06

score 0 · Answer 3 · answered Jun 01 '17 at 07:51

std::vector<long> File_read(){
    vector<long> numbers;
    ifstream my_file("H:\\Sanduni_projects\\testing\\test.txt", 
     std::ifstream::binary);
    if (my_file) {
        string line;

        for (int i = 0; i < frames_sec; i++){
            getline(my_file, line);
            numbers.push_back(std::stol(line));
        }

        if (my_file)
            std::cout << "all characters read successfully." << endl;
        else
            std::cout << "error: only " << my_file.gcount() << " could be read" << endl;
        my_file.close();

    }
    else{
        cout << "File can not be opened" << endl;
    }
    return numbers;
}

score 0 · Answer 4 · edited Jun 01 '17 at 08:11

Although the someone gives the answers that works correctly, I want to share my code.

#include <memory>
#include <iostream>
#include <string>
#include <fstream>
using namespace std;
#define MAX_SIZE 4096
class FileRead
{
public:
    FileRead(string path) :_file(path)
    {
        Reset();
    }
    void Reset()
    {
        memset(_buff, 0, MAX_SIZE);
    }
    string ReadLine()
    {
        if (!_file.is_open())
        {
            cout << "error open file" << endl;  
            return "";
        }
        if (!_file.eof())
        {
            Reset();
            _file.getline(_buff,MAX_SIZE);
            return string(_buff);
        }
        else
        {
            cout << "read file finished." << endl;
            return "";
        }       
    }

private:
    ifstream _file;
    string _line;
    char _buff[MAX_SIZE];
};

int _tmain(int argc, _TCHAR* argv[])
{   
    FileRead fr("H:\\Sanduni_projects\\testing\\test.txt");
    string line;
    while (!(line = fr.ReadLine()).empty())
    {
        //do some compare..
    }
    return 0;    
}

Caleth · Answer 5 · 2017-06-01T08:48:56.920

The other answers are correct about how return works, but there is something that acts how you thought return acted.

using string_coro = boost::coroutines::asymmetric_coroutine<std::string>

void File_read(string_coro::push_type & yield) {
    std::ifstream my_file("H:\\Sanduni_projects\\testing\\test.txt", std::ifstream::binary);
    if (my_file) {
        string line;

        for (int i = 0; i < 25; i++){
            getline(my_file, line);
            yield (line);
        }

        if (my_file)
            std::cout << "all characters read successfully." << std::endl;

        my_file.close();
    }
}

Which is used like this

string_coro::pull_type(File_read) strings;
for (const std::string & s : strings)
    std::cout << s << endl;

How to return a string line by line in a function?

5 Answers5

Never return unconditionally inside a loop.

No need to reinvent stuff

Find a match