Reading a collection of strings from the user

Question

I was trying to write a program that will read a collection of strings from the user, and then end the moment it encounters a ".". So then I write a do-while loop.

I came across a something like this:

string temp;
vector<string> params;
do
{
 cin >> temp;
 if (temp == ".")
 break;
 params.push_back(temp);
} while (temp != ".");

I realized that no matter what, the loop will always end from within its body -- which is the exact result that want.

But there something about piece of code that smells fishy. Are there any better ways?

Another thing to note: I don't want the "." to be pushed onto the vector, hence that's why I added that little if (temp == ".") break; statement.

Answer 1

Don't forget to check the stream status for errors or EOF.

while (cin >> temp && temp != ".")
{
 params.push_back(temp);
}

---

EDIT: You do not necessarily need to invent your own break condition. There's one already — end of file. You can just read strings until you reach it. This way, your program will also work with non-interactive input nicely. To generate an end of file on a terminal, type Ctrl+D on Unix/Linux and Ctrl+Z on Windows.

while (cin >> temp)
{
 params.push_back(temp);
}

• Christopher Creutzig: Almost an extremely nice solution – it just lacks the {} which I strongly prefer to see even for one-line bodies.
• Andrey Vihrov: @Christopher: This is a matter of personal style. I will nevertheless edit this answer for better clarity.
• Christopher Creutzig: Yes it is, and I didn't mean it any other way. (But note that some versions of gcc will flag such "missing braces" with -Wall, which really should be among the standard settings for any developer.)
• Jerry Coffin: Almost an extremely nice solution -- it just has those ugly, completely unnecessary {} polluting and obfuscating otherwise nice code. :-)
• Konrad Rudolph: @Christopher Which versions of GCC flag this? I always use -Wall (and more), never put the braces and have never seen this warning.

Answer 2

I prefer:

cin >> temp;
while (temp != ".")
{
 params.push_back(temp);
 cin >> temp;
}

Answer 3

string temp;
vector<string> params;
while (true)
{
 cin >> temp;
 if (temp == ".")
 break;
 params.push_back(temp);
}

That test - true in my case, temp != "." in yours, never really gets run, except when it's true. So it might as well be true.

• mmocny: This would be my suggestion, with the modification of using for(;;) rather than do {} while(true); I prefer it to Michael K's answer because it doesn't repeat the "body" of the loop (cin>>temp) and is no slower (while still needs to execute a branch). If the "body" of the loop was more than one line, it would more clearly be code duplication.
• Hosam Aly: I would suggest that while(true) (without do) would be clearer for intentions, even for less experienced programmers, than for(;;) or do while(true).
• Carl Manaster: Good call, @Hosam Aly; I have updated with your suggestion. Thanks!

Answer 4

I prefer to build some infrastructure that will make the rest of the code trivial. The infrastructure may be a little extra work, but the long-term savings can be substantial. In this case, it takes the form of a special iterator that allows you to specify the "sentinel" that will end the input. It acts like a normal istream_iterator, except that you specify the sentinel value when you construct the "end of range" iterator.

// sentinel_iterator.h
#pragma once
#if !defined(SENTINEL_ITERATOR_H_)
#define SENTINEL_ITERATOR_H_
#include <istream>
#include <iterator>
template <class T,
 class charT=char,
 class traits=std::char_traits<charT>,
 class distance = ptrdiff_t>
class sentinel_iterator :
 public std::iterator<std::input_iterator_tag,distance,void,void,void>
{
 std::basic_istream<charT,traits> *is;
 T value;
public:
 typedef charT char_type;
 typedef traits traits_type;
 typedef std::basic_istream<charT,traits> istream_type;
 sentinel_iterator(istream_type& s)
 : is(&s)
 { s >> value; }
 sentinel_iterator(T const &s) : is(0), value(s) { }
 const T &operator*() const { return value; }
 const T *operator->() const { return &value; }
 sentinel_iterator &operator++() {
 (*is)>>value;
 return *this;
 }
 sentinel_iterator &operator++(int) {
 sentinel_iterator tmp = *this;
 (*is)>>value;
 return (tmp);
 }
 bool operator==(sentinel_iterator<T,charT,traits,distance> const &x) {
 return value == x.value;
 }
 bool operator!=(sentinel_iterator<T,charT,traits,distance> const &x) {
 return !(value == x.value);
 }
};
#endif 

With that in place, reading the data becomes trivial:

#include <string>
#include <vector>
#include <algorithm>
#include <iostream>
#include "sentinel_iterator.h"
int main() { 
 // As per spec, read until a "." is entered:
 std::vector<std::string> strings(
 sentinel_iterator<std::string>(std::cin), 
 sentinel_iterator<std::string>("."));
 // It's not restricted to strings either. Read numbers until -1 is entered:
 std::vector<int> numbers(
 sentinel_iterator<int>(std::cin),
 sentinel_iterator<int>(-1));
 // show the strings:
 std::copy(strings.begin(), strings.end(),
 std::ostream_iterator<std::string>(std::cout, "\n"));
 // show the numbers:
 std::copy(numbers.begin(), numbers.end(),
 std::ostream_iterator<int>(std::cout, "\n"));
 return 0;
}

Given an input of:

This is a string .
1 2 3 5 -1

It produces an output of:

This
is
a
string
1
2
3
5

It should work for essentially any type that defines a stream extractor and testing for equality (i.e., saying x==y will compile and produce meaningful results).

• TemplateRex: I like the idea (and +1). In contrast, std::istream_iterator is comparing stream pointers instead of read values. This has a small efficiency cost because both the constructor and operator++ need an extra if() to see if the read has failed in order to null out the stream pointer. Your sentinel_iterator doesn't need that. Could you also use it to read from std::cin until a read fails?

Answer 5

If we are not talking about language-specific details then I would prefer something like this:

// this is inspired by LINQ and C#
var params = Enumerable.Generate<string>(() => {string temp; cin >> temp; return temp; })
 .TakeWhile(s => s != ".")
 .ToVector();

Where Enumerable.Generate() is some lambda which reads data from cin. Generally answering the question 'how to use breaks?' I think breaks should not be used, at least not in such trivial scenarios.

• mmocny: I am not a fan of this particular syntax, but I like the methodology.