52

Common std::cin usage

int X;
cin >> X;

The main disadvantage of this is that X cannot be const. It can easily introduce bugs; and I am looking for some trick to be able to create a const value, and write to it just once.

The naive solution

// Naive
int X_temp;
cin >> X_temp;
const int X = X_temp;

You could obviously improve it by changing X to const&; still, the original variable can be modified.

I'm looking for a short and clever solution of how to do this. I am sure I am not the only one who will benefit from a good answer to this question.

// EDIT: I'd like the solution to be easily extensible to the other types (let's say, all PODs, std::string and movable-copyable classes with trivial constructor) (if it doesn't make sense, please let me know in comments).

Graham Borland
  • 60,055
  • 21
  • 138
  • 179
Bartek Banachewicz
  • 38,596
  • 7
  • 91
  • 135
  • is the variable you wish to initialize a class member? – oopsi Sep 05 '12 at 10:43
  • 3
    Make the first two lines a function returning `int` :) `const int X = read_cin();` – Sergey Kalinichenko Sep 05 '12 at 10:43
  • Put first 2 lines of your solution in a function and return `X_temp`. – iammilind Sep 05 '12 at 10:53
  • 1
    Changing `const int` to `const int&` won't improve anything. Second one is almost exactly the same as `const int * const` so you will be copying `sizeof(int*)` and in `const int` you will be copying `sizeof(int)`, so probably exactly the same amount of data. Using reference to `int` has no sense - you probably shouldn't use reference to any of POD-s. – Pawel Zubrycki Sep 05 '12 at 10:54
  • @dasblinkenlight This requires a function for every type I'd like to read. – Bartek Banachewicz Sep 05 '12 at 10:54
  • @PawelZubrycki Please read the Edit. – Bartek Banachewicz Sep 05 '12 at 10:54
  • 4
    I personally like the "naive solution" you put. The value you read in from the user is manifestly NOT a constant, then you are explicity copying it's value into another value which you promise won't change by marking it const. It's slightly ugly but seems to exactly fit what's happening. – jcoder Sep 05 '12 at 11:02
  • 3
    Your "naive" solution is how it should be done. Nobody benefits from a huge bombastic class interface for performing such a simple, mundane task. If you find yourself writing long, complicated functions merely to set a variable, that's a certain sign telling that something has gone terribly wrong in the program design. – Lundin Sep 05 '12 at 11:03
  • But using the naive way, I have to create a temp variable for each type I am going to read from the stream: `string TempString; int TempInt;` and so on... I just thought one could encapsulate it in a function, so it will be unnamed. – Bartek Banachewicz Sep 05 '12 at 11:06
  • 4
    @BartekBanachewicz "This requires a function for every type I'd like to read." Make it a `read_cin()` then :) – Sergey Kalinichenko Sep 05 '12 at 11:15

6 Answers6

25

I'd probably opt for returning an optional, since the streaming could fail. To test if it did (in case you want to assign another value), use get_value_or(default), as shown in the example.

template<class T, class Stream>
boost::optional<T> stream_get(Stream& s){
  T x;
  if(s >> x)
    return std::move(x); // automatic move doesn't happen since
                         // return type is different from T
  return boost::none;
}

Live example.

To further ensure that the user gets no wall-of-overloads presented when T is not input-streamable, you can write a trait class that checks if stream >> T_lvalue is valid and static_assert if it's not:

namespace detail{
template<class T, class Stream>
struct is_input_streamable_test{
  template<class U>
  static auto f(U* u, Stream* s = 0) -> decltype((*s >> *u), int());
  template<class>
  static void f(...);

  static constexpr bool value = !std::is_void<decltype(f<T>(0))>::value;
};

template<class T, class Stream>
struct is_input_streamable
  : std::integral_constant<bool, is_input_streamable_test<T, Stream>::value>
{
};

template<class T, class Stream>
bool do_stream(T& v, Stream& s){ return s >> v; }
} // detail::

template<class T, class Stream>
boost::optional<T> stream_get(Stream& s){
  using iis = detail::is_input_streamable<T, Stream>;
  static_assert(iis::value, "T must support 'stream >> value_of_T'");
  T x;
  if(detail::do_stream(x, s))
    return std::move(x); // automatic move doesn't happen since
                         // return type is different from T
  return boost::none;
}

Live example.

I'm using a detail::do_stream function, since otherwise s >> x would still be parsed inside get_stream and you'd still get the wall-of-overloads that we wanted to avoid when the static_assert fires. Delegating this operation to a different function makes this work.

Xeo
  • 129,499
  • 52
  • 291
  • 397
  • 2
    I’d claim that this is the only acceptable solution. But well, I’m weird. – Konrad Rudolph Sep 05 '12 at 11:48
  • 3
    @KonradRudolph: That's a good way to define `weird`, just as much as the post is a great example of `overkill` :-) – Kerrek SB Sep 05 '12 at 14:29
  • @Kerrek Structuring your code flow so that you have `const` objects? I don’t consider that in the least bit overkill. – Konrad Rudolph Sep 05 '12 at 14:37
  • 3
    @KonradRudolph: What do you do with an optional const?! "Here's a constant... maybe"? – Kerrek SB Sep 05 '12 at 15:15
  • @KerrekSB “Here’s a constant which may contain a value”. Why is that weird? It isn’t – other languages do that all the time (Haskell …) and C++ has the huge boon of allowing this as well. – Konrad Rudolph Sep 05 '12 at 15:21
  • @Kerrek: Was the "overkill" directed at the `static_assert`? If yes, I advise you [check the error message](http://chat.stackoverflow.com/transcript/10?m=5231326#5231326) if your `T` does *not* support input-streaming. I very much consider this *not* to be overkill, though you could maybe blame the implementation for spewing this huge list out in the first place... I wouldn't do that though, since it lets you discover if you just made a typo or something else when you do implement an overloaded function (it also says why that function was SFINAE'd out if that happened). – Xeo Sep 05 '12 at 16:02
  • 5
    @Xeo: It was meant somewhat tongue-in-cheek. I do appreciate the finesse of the solution. I was just amused at how big a solution the deceptively simple desire to initialize `X` had precipitated :-) – Kerrek SB Sep 05 '12 at 19:26
  • Anyway, I am accepting it as the only answer that is perfectly correct. – Bartek Banachewicz Sep 06 '12 at 12:08
20

You could make use of lambdas for such cases:

   const int x = []() -> int {
                     int t;
                     std::cin >> t;
                     return t;
                 }();

(Note the extra () at the end).

Instead of writing a separate functions, this has the advantage of not having to jump around in your source file, when reading the code.

Edit: Since in the comments it was stated that this goes against the DRY rule, you could take advantage of auto and 5.1.2:4 to reduce type repetition:

5.1.2:4 states:

[...] If a lambda-expression does not include a trailing-return-type, it is as if the trailing-return-type denotes the following type:

  • if the compound-statement is of the form

    { attribute-specifier-seq(opt) return expression ; }

    the type of the returned expression after lvalue-to-rvalue conversion (4.1), array-to-pointer conversion (4.2), and function-to-pointer conversion (4.3);

  • otherwise, void.

So we could alter the code to look like this:

   const auto x = [] {
                     int t;
                     std::cin >> t;
                     return t;
                  }();

I can't decide if that is better though, since the type is now "hidden" within the lambda body...

Edit 2: In the comments it was pointed out, that just removing the type name where it is possible, does not result in a "DRY-correct" code. Also the trailing-return-type deduction in this case is currently actually an extension of MSVC++ as well as g++ and not (yet) standard.

lx.
  • 2,317
  • 1
  • 22
  • 32
  • Hm, can you skip the first set of `()` just after the `[]`? – Bartek Banachewicz Sep 05 '12 at 10:55
  • 1
    @Bartek: Not if you want to provide a trailing return type. – Xeo Sep 05 '12 at 10:56
  • wow, I like lambda expressions a lot... but that just looks ugly! (Not a comment on the quality of the answer at all just how the code ends up looking visibly) – jcoder Sep 05 '12 at 10:59
  • I really like the thinking, but it seems to violate DRY rule for me. – Bartek Banachewicz Sep 05 '12 at 11:01
  • @Bartek When you have to specify a trailing return type,you also have to write out the lambda-declarator. Unfortunately I can't find the exact paragraph in the standard right now. It could be §5.1.2.4. In this case you might also be able to omit the trailing return type altogether. To be honest, my knowledge of lambas is not that deep yet. – lx. Sep 05 '12 at 11:03
  • 1
    @BartekBanachewicz As for the DRY: You could use `auto`, but that'd only save you one repetition. – lx. Sep 05 '12 at 11:16
  • 2
    @lx. it's in the grammar in 5.1.2:1; *lambda-declarator* is optional, but if included it must contain a *parameter-declaration-clause* before the optional *trailing-return-type*. The semantics of an omitted *lambda-declarator* or an omitted *trailing-return-type* are defined in 5.1.2:4. – ecatmur Sep 05 '12 at 11:53
  • The return type can't be omitted if there's more than one statement in the lambda body, see http://stackoverflow.com/questions/6240894/is-there-a-reason-on-not-allowing-lambdas-to-deduce-the-return-type-if-it-contai – ecatmur Sep 05 '12 at 16:47
  • As I understood (and using with VC11 tested) it, the return type can be deduced if there is only one return statement within the lambda body. See also [MSDN:Lambda Expression Syntax](http://msdn.microsoft.com/en-us/library/dd293603.aspx) – lx. Sep 05 '12 at 17:30
  • @lx. that's an extension that both MSVC and g++ implement, but it's not (yet) standard C++. Issue 975 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#975 is currently under review and looks likely to be accepted. The current language still requires a body of the form `{ attribute-specifier-seqopt return expression ;}`. – ecatmur Sep 05 '12 at 18:55
  • 3
    The real spirit of DRY is to refactor the pattern into a function such that you end up with e.g. `const auto i = extract(std::cin);`, not that you make the pattern shorter. – Luc Danton Sep 05 '12 at 19:10
  • @ ecatmur, @LucDanton : Ah, I see. Thanks both of you. I'm not sure how to edit my answer now, though.. – lx. Sep 06 '12 at 05:50
14

A slight tweak to lx.'s lambda solution:

const int x = [](int t){ return iss >> t, t; }({});

Significantly less DRY violation; can be eliminated entirely by changing const int x to const auto x:

const auto x = [](int t){ return iss >> t, t; }({});

One further improvement; you can convert the copy into a move, since otherwise the comma operator suppresses the optimisation in 12.8:31 (Move constructor suppressed by comma operator):

const auto x = [](int t){ return iss >> t, std::move(t); }({});

Note that this is still potentially less efficient than lx.'s lambda, as that can benefit from NRVO whereas this still has to use a move constructor. On the other hand an optimising compiler should be able to optimise out a non-side-effect-bearing move.

Community
  • 1
  • 1
ecatmur
  • 152,476
  • 27
  • 293
  • 366
6

You can call a function to return the result and initialize in the same statement:

template<typename T>
const T in_get (istream &in = std::cin) {
    T x;
    if (!(in >> x)) throw "Invalid input";
    return x;
}

const int X = in_get<int>();
const string str = in_get<string>();

fstream fin("myinput.in",fstream::in);
const int Y = in_get<int>(fin);

Example: http://ideone.com/kFBpT

If you have C++11, then you can specify the type only once if you use the auto&& keyword.

auto&& X = in_get<int>();
ronalchn
  • 12,225
  • 10
  • 51
  • 61
6

Sure you can do this be just constructing a temporary istream_iterator. For example:

const auto X = *istream_iterator<int>(cin)

It's worth pointing out here that you're abandoning all hope of error checking when you do this. Which generally in the taking of input from a user would not be considered the wisest... but hey, maybe you've curated this input somehow?

Live Example

Jonathan Mee
  • 37,899
  • 23
  • 129
  • 288
  • 1
    Pretty amazing that no one has thought about that for 5 years! – Bartek Banachewicz Mar 30 '18 at 20:59
  • @BartekBanachewicz I can only guess that this primarily relates to the lack of error checking? I use this a lot with `istringstream`s and `ifstream`s that I can control. But... yeah I dunno it does seem like the most obvious answer? – Jonathan Mee Mar 30 '18 at 21:16
3

I'm assuming that you will want to initialize a global variable, since for a local variable it just seems like a very awkward choice to forgo three lines of plain and understandable statements in order to have a constant of questionable value.

At the global scope, we can't have errors in the initialization, so we'll have to handle them somehow. Here are some ideas.

First, a templated little construction helper:

template <typename T>
T cinitialize(std::istream & is) noexcept
{
    T x;
    return (is && is >> x) ? x : T();
}

int const X = cinitialize<int>(std::cin);

Note that global initializers must not throw exceptions (under pain of std::terminate), and that the input operation may fail. All told, it's probably pretty bad design to initialize global variables from user input in such a fashion. Perhaps a fatal error would be indicated:

template <typename T>
T cinitialize(std::istream & is) noexcept
{
    T x;

    if (!(is && is >> x))
    {
        std::cerr << "Fatal error while initializing constants from user input.\n";
        std::exit(1);
    }

    return x;
}

Just to clarify my position after some discussion in the comments: In a local scope I would never resort to such an awkward crutch. Since we're processing external, user-supplied data, we basically have to live with failure as part of the normal control flow:

void foo()
{
    int x;

    if (!(std::cin >> x)) { /* deal with it */ }
}

I leave it up to you to decide whether that's too much to write or too hard too read.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • It's more like cininitialize_int. Something more generic would be better. – Bartek Banachewicz Sep 05 '12 at 10:49
  • @BartekBanachewicz You can easily make it a template and replace the `-1` by `T()` (which would also be better in the `int` version, since `-1` is just a special value without any reasonable meaning. Ok, `0` isn't much better, but at least it's the result of zero/value-initialization). – Christian Rau Sep 05 '12 at 10:57
  • The fatal error seems a little excessive. Perhaps take an optional extra parameter for an error flag, if you really don't want exceptions? That will mean you have to have a mutable variable somewhere, but a mutable error flag seems much more tolerable to me. On the other hand, order of initialisation of globals is too crazy for such a way to be sane anyway, imho, and thus perhaps throwing an exception is acceptable. – Cactus Golov Sep 05 '12 at 10:58
  • 3
    @AntonGolov: Throwing an exception from a global initializer terminates the program. – Kerrek SB Sep 05 '12 at 11:07
  • 3
    @KerrekSB: Global initialisers are allowed to throw exceptions (for some value of "allowed" - it will cause a call to `std::terminate`); and there's no indication that the OP wants to initialise globals this way. – Mike Seymour Sep 05 '12 at 11:07
  • 1
    Btw, I don't think you need to double check if the stream itself is valid, this will be done inside `operator>>` anyways. – Xeo Sep 05 '12 at 11:22
  • @Xeo: I know, I know... I was in a very boolean mood. And ever since C++11 I like adding `&&` wherever I can. Does it show? – Kerrek SB Sep 05 '12 at 11:25
  • 1
    Stop the FUD about exceptions. Throwing an exception during dynamic initialization will **NOT** cause anything bad to happen. *Letting the exception escape* will. As such, there is no need for every caller to bear the burden of using such a subpar error reporting method. Use exceptions, and if someone wants to use the function in a SSDO initializer *they* will take care to e.g. wrap the call in a try-catch block in a lambda. I know you're better than irrationally being afraid of exceptions. – Luc Danton Sep 05 '12 at 19:21
  • @LucDanton: My function is designed for the one and only purpose of implementing the OP's requirements and answering his question. It isn't a universal tool for doing something you can already do. So I put everything I need *for the OP's purpose* into the function... is that so bad? That's also why I'm not going wild with generalisations. It wasn't needed, and yes I could, but sometimes you shouldn't put more than one sock on one foot, as someone once said. – Kerrek SB Sep 05 '12 at 19:23
  • 1
    The OP does not explicitly say that this is about initializing namespace scope variables. Regardless, a future visitor might take from your answer at face value and really hold that using exceptions in SSDO initializers is intractable. – Luc Danton Sep 05 '12 at 19:28
  • @LucDanton: Well... that would be unfortunate, and I hope that saying "global initializer" makes this special condition clear. I really don't think the answer (not the comments!) is *that* misleading. And yes, I *could* also describe initialization in other contexts but again, you can only walk so far on one sock... – Kerrek SB Sep 05 '12 at 19:30
  • 1
    I don't care about your socks. The point is that throwing during dynamic initialization is fine, as long as you catch. What you're saying flies plainly in the face of that, and fixing that does not mean adding socks. – Luc Danton Sep 05 '12 at 19:32
  • Hmm, a fatal error output followed by `std::exit` is indeed much better than a `std::terminate` from an uncaught exception. – Christian Rau Sep 06 '12 at 08:23