Using extraction operator on a stream for a function argument

I am working on the beginning of some code and I am having problems with this snippet:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
while (getline(configFile, buffer)) {
                if ('#' == buffer[0]) // if the first character of the buffer is a #, ignore it
                        continue;
                else if (string::npos == buffer.find('=')) { // if '=' is not found in string buffer
                        atom* n = new atom();
                        while (istringstream(buffer).good()) {
                                n->setType(istringstream(buffer) >>);
                                n->setID(istringstream(buffer) >>);
                                n->setX(istringstream(buffer) >>);
                                n->setY(istringstream(buffer) >>);
                                n->setZ(istringstream(buffer) >>);
                        }
                        configAtoms.push_back(*n);
                }
                else {
                        key = buffer.substr(0, buffer.find('='));
                        value = buffer.erase(0, buffer.find('=') + 1);
                        configMap[key] = value;
                }
        }
}


where

 
istringstream(buffer) >>


is a pseudo-code placeholder for a function that would return the next type independent token, as if I had just used the extraction operator on a stream. It would be possible to read the next token in to a variable every other line and then set the object member with the .set function, but this will make the code twice as long. Eventually, the atom objects will have upwards of 20 members to read in per line of the configuration file, so it would be much cleaner if I could pull a token from the buffer string and pass it directly to the function. Is this posssible?
n->setType(istringstream(buffer) >>);
this creates an std::istringstream object with the entire std::string buffer (the ctor is not parsed on white-space), so all the istringstream(buffer) calls try to read the entire contents of buffer into data-members type, ID, X, Y, Z which is certainly not what you want.
a more robust way of doing this might be:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
std::istringstream stream{buffer};
while (stream)
{
	std::string tempType{};
	std::string tempID{};
	size_t tempX{}, tempY{}, tempZ{};
	//assumes the 'token' or delimiter is white-space, you can supply custom delimiters here as well 
	stream >> tempType >> tempID >> tempX >> tempY >> tempZ;
	if(configFile && stream)//check file and stream are still valid
	{
		atomVec.push_back(std::move(atom(tempType, tempID, tempX, tempY, tempZ)));
		//where atomVec is some container of Atom objects like a std::vector<Atom> declared outside this scope ...
		//... so as to be avalalble to the rest of the program after file read is complete 
	}
}

also variable declared on line 16 is unused
Last edited on
Abbotta4 wrote:
the atom objects will have upwards of 20 members to read in [...] it would be much cleaner if I could pull a token from the buffer string and pass it directly to the function. Is this posssible?

sounds like your atom needs its own operator>>. Then you may even be able to eliminate the public setters (setID, setX, setY), which would be a huge improvement.
Or use a constructor as in gunnerfunner's example, that's good too.

But use case notwithstanding, the thing you're imagining is called input iterator:
1
2
3
4
5
6
7
8
9
    atom n;
    std::istringstream is(buffer);
    std::istream_iterator<std::string> it(is), end;
    if(it != end) n.setType(*it++); // setters are awful
    if(it != end) n.setID(*it++); // don't do this
    if(it != end) n.setX(*it++); // use a constructor like in gunnerfunner's example
    if(it != end) n.setY(*it++); // or operator>>(istream&, atom)
    if(it != end) n.setZ(*it); // so you can just do "if(is >> n) ca.push_back(n);"
    if(it != end) configAtoms.push_back(n); 
Last edited on
A constructor would clean up the many newlines required, but I would still have to use intermediary variables. So when the atom class becomes large enough, am I just stuck with declaring 20 temporary variables in the while block?

Also, isn't key being used in line 18 for the index names?
when the atom class becomes large enough, am I just stuck with declaring 20 temporary variables

afraid so. In that case overloading the insertion operator, as Cubbi suggests, might reduce typing as you wouldn't need to declare the temporaries ONLY IF your data members are public. otherwise you'd have to again use the temporaries or setters. for public data members the overload might look like:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
std::istream& operator >> (std::istream& configFile, Atom& a)
{
    std::string buffer{};
    while (getline(configFile, buffer))
    {
        if (buffer[0] == '#') continue;
        else if( buffer.find('=') == std::string::npos)
        {
            std::istringstream is(buffer);
            while (is)
            {
                is >> a.Type >> a.ID /* ... */ ;
            }
            if(configFile)configAtoms.push_back(std::move(a));
        }
        else
        {
            //...
        }
    }
}
for public data members the overload might look like:

I would strongly suggest just making your overloaded operator responsible for extracting a single Atom from a stream. The result of the code proffered by gunnerfunner should be surprising to anyone that is familiar with overloaded extraction operators and that is not a good thing.

Anytime you find yourself ignoring the value of a passed in variable and using it as a makeshift local, there's probably something wrong with the design of the function.



In that case overloading the insertion operator, as Cubbi suggests, might reduce typing as you wouldn't need to declare the temporaries ONLY IF your data members are public.

Hello, friend.
http://en.cppreference.com/w/cpp/language/friend
indeed, that's what friends are for after all. thanks
edit:
OP: cire is quite right, I was focussing solely on the else-if bit but there are other branches as well so this would not work here
Last edited on
Topic archived. No new replies allowed.