5
\$\begingroup\$

I am a newbie to C++ and I want to do some cool stuff with it. Here I have implemented my own getline function which reads from a file. so what do you think of this getline function?

#include <iostream>
#include <fstream>
using namespace std;

class Foo {
private:
    char ch;
public:
    void getline(fstream& in, string& word, char delimeter ='\n'){
        word = "";
        while(in.get(ch)){
            word += ch;
            if(ch==delimeter)
                break;
        }
    }
};
int main(){
    Foo one;
    string temp;
    fstream obj("file.txt", ios::in | ios::out | ios::trunc );

    obj<<"this is our$ world";
    obj.seekg(0);
    one.getline(obj, temp,'$');
    cout<<temp<<endl;
}
\$\endgroup\$
6
\$\begingroup\$

Free the Functions!

First, as others have pointed out, this gains nothing from being a class--it probably makes more sense as a free function.

Good Inheritors Would Rather Switch than Fight!

Second, this specifies that its argument must be an fstream. This seems rather pointless. One of the major points of streams is that most functions can (and do) handle various kinds of streams by working with the base classes--usually std::istream and std::ostream. This lets your code work with any stream that provides the interface you need (in this case, just the ability to read, so an istream suffices).

Return Status

With those changed, the big shortcoming I see in this getline compared to std::getline is that it fails to return anything that can be interpreted as the status of the attempt at reading.

For example, in a typical case I might read and process all the lines of text from a file using a loop something like this:

std::ifstream input("input.txt");
std::string line;

while (std::getline(input, line))
    process(line);

If I want to read all the lines from a file with this getline, I guess I can do something like this:

while (getline(input, line), input.good())
    process(line);

That should work, but it's a bit clumsy at best, and sufficiently strange that most people would probably find it more difficult to understand. I think it would be better to do like most I/O routines, and return a reference to the stream you read from:

std::istream &getline(std::istream& in, string& word, char delimeter ='\n'){
    char ch;

    word = "";
    while(in.get(ch)){
        word += ch;
        if(ch==delimeter)
            break;
    }
    return in;
}

Spelling

The word you're spelling as "delimeter" is actually spelled "delimiter".

Delimiter Usage

Your code has behavior that differs from std::getline in a way I think reduces its utility. std::getline normally reads the delimiter from the stream, but does not include that delimiter in the output string (where your code does include the closing delimiter in the output string). At least when I've used it, including that delimiter wouldn't be very useful--I'd usually have to delete it if I read a string with this code. I'd probably omit it, and (if necessary) provide an overload that saved it (but still probably separately from the string it read).

Delimiter Specification

Probably the single biggest problem I have with std::getline is that you can only specify one specific character as the delimiter. If I were going to write one, I think I'd allow the user to pass something like a string, so they could (for example) pass "\t\r\n\v" as the delimiter, to have the function read to the next white-space.

Although it's more advanced, you could go even further, and let the user specify the delimiter as a regular expression, so they could specify arbitrarily complex patterns instead of just a few specific characters.

\$\endgroup\$
4
\$\begingroup\$

This doesn't need to be in a class. It would do just fine as a free function:

void getline(fstream& in, string& word, char delim ='\n'){
    char ch;
    word = "";
    while(in.get(ch)){
        word += ch;
        if(ch==delim)
            break;
    }
}
\$\endgroup\$
4
\$\begingroup\$

I would cleanup the logic slightly by using a for-loop or a do-while:

void getline(std::fstream& in, std::string& word, char delim ='\n')
{
    char ch;

    do
    {
        in.get(ch);
        word += ch;
    }
    while(ch != delim);
}

I think this version reads more nicely by avoiding the break-statement. By the way, I also wouldn't assign word to be empty inside the function: this could be up to the user. Either you pass in an empty string if you want to start fresh, but maybe you do want to append, which you can't do with your current version.

As also mentioned, there is no reason to have the functionality in a class. Just make it a free function.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.