Input class

A contribution of my own.
This is a simple input class whose sole purpose is to reduce the amount of lines and work you need to do to acquire input from the console (via cin).
The class is made to work with both char and wchar_t, as well as any custom class that has an overloaded << operator.
The code is made to work with both exceptions and return values. The functions that return the value directly throws an exception if it goes wrong. The functions that take a reference to a variable to store to and returns a bool will not throw.
Dependencies:
- Boost
- C++ Standard Library
Source:

Code:

---

// Input.h

#ifndef INPUT_20081018_H
#define INPUT_20081018_H

#include <iostream>
#include <boost/lexical_cast.hpp>
#include <boost/integer_traits.hpp>
#include <Stuff/Traits.h>

namespace Stuff
{
        template<typename CharT> struct InputTraits
        {
                static std::basic_ostream<char>& out_stream;
                static std::basic_istream<char>& in_stream;
                typedef std::string StorageType;
                typedef typename boost::add_reference
                <
                        typename boost::add_const<typename integer_traits<CharT>::BasicType>::type
                >::type CharType;
        };
        template<typename CharT> std::basic_ostream<char>& InputTraits<CharT>::out_stream = std::cout;
        template<typename CharT> std::basic_istream<char>& InputTraits<CharT>::in_stream = std::cin;

        template<> struct InputTraits<const char*>: public InputTraits<char>
        {
                typedef const char* CharType;
        };

        template<> struct InputTraits<const wchar_t*>
        {
                static std::basic_ostream<wchar_t>& out_stream;
                static std::basic_istream<wchar_t>& in_stream;
                typedef std::wstring StorageType;
                typedef const wchar_t* CharType;
        };
        std::basic_ostream<wchar_t>& InputTraits<const wchar_t*>::out_stream = std::wcout;
        std::basic_istream<wchar_t>& InputTraits<const wchar_t*>::in_stream = std::wcin;

        template<> struct InputTraits<wchar_t*>: public InputTraits<const char*> {};
        template<> struct InputTraits<char*>: public InputTraits<const char*> {};

        #ifdef UNICODE
                typedef wchar_t* StrType_t;
        #else
                typedef char* StrType_t;
        #endif

        template<typename CharT = StrType_t, template<typename> class Traits = InputTraits> class CInput
        {
        private:
                typedef Traits<CharT> Traits_;
        public:
                CInput() {}
                CInput(CharT strInputQ)
                {
                        *this << strInputQ;
                }

                template<typename Type> bool ask_and_get(CharT strInputQ, Type& Answer) // does not throw
                {
                        ask(strInputQ);
                        return get(Answer);
                }

                template<typename Type> bool get(Type& Answer) // does not throw
                {
                        try
                        {
                                Answer = boost::lexical_cast<Type>(m_Answer);
                        }
                        catch (boost::bad_lexical_cast)
                        {
                                return false;
                        }
                        return true;
                }

                CInput& operator << (typename Traits<CharT>::CharType rhs)
                {
                        Traits<CharT>::out_stream << rhs;
                        return *this;
                }

                template<typename DstType> CInput& operator >> (DstType& rhs)
                {
                        get_answer();
                        rhs = boost::lexical_cast<DstType>(m_Answer);
                        return *this;
                }

        private:
                void get_answer()
                {
                        std::getline(Traits_::in_stream, m_Answer);
                }

                typename Traits_::StorageType m_Answer;
        };
}

#endif // INPUT_20081018_H

// Traits.h

#ifndef TRAITS_20081019_H
#define TRAITS_20081019_H

#include <boost/type_traits.hpp>
#include <stdint.h>
namespace Stuff
{
        template<typename T> struct ParamType
        {
                typedef typename boost::remove_cv
                <
                        typename boost::remove_const
                        <
                                typename boost::remove_pointer
                                <
                                        typename boost::remove_reference<T>::type
                                >::type
                        >::type
                >::type BasicType;
        };

        template<typename T> struct integer_traits: public boost::integer_traits<T>, public ParamType<T>
        {
                static const bool is_array = false;
                static const uint32_t array_dimensions = 0;
        };

        template<typename T> struct integer_traits<T[]>: public boost::integer_traits<T>, public ParamType<T>
        {
                static const bool is_array = true;
                static const uint32_t array_dimensions = 1;
        };
}

#endif // TRAITS_20081019_H

---

The class works in 3 steps:
1) Print question to user (can be done manually).
2) Fetch answer from user (stored internally).
3) Validate input and return it to the program.
There are a few simple functions.
The empty constructor just constructs the object.
The second constructor executes step 1 & 2.
The function ask executes steps 1 & 2.
The function ask_and_get performs all 3 steps.
The function get performs step 3.

Example:

Code:

---

#include <Stuff/Input.h>
#include <iostream>

class foo;
template<typename T> std::basic_ostream<T>& operator << (std::basic_ostream<T>& lhs, const foo& rhs);

class foo
{
public:
        std::string x;
        template<typename T> friend std::basic_ostream<T>& operator << <> (std::basic_ostream<T>& lhs, const foo& rhs);
};

template<typename T> std::basic_ostream<T>& operator << (std::basic_ostream<T>& lhs, const foo& rhs)
{
        lhs << rhs.x;
        return lhs;
}

int main()
{
        foo f;
        f.x = "Enter an integer: ";

        Stuff::CInput<int> input;
        int x;
        while ( !input.ask_and_get(f, x) )
        {
                std::cout << "That is not an integer.\n";
        }
        std::cout << "You entered: " << x << "\nHave a nice day.\n";
}

---

Output:
Enter an integer: a
That is not an integer.
Enter an integer: ,
That is not an integer.
Enter an integer: 555
You entered: 555
Have a nice day.
Press any key to continue . . .

Criticism is welcome!

So I can steal it and put it in my MFC text adventure framework? I detest writing input classes and yours looks good.

The code is open and all that. A small thank you or something would be nice, but otherwise you can use it as you wish.

>> "You bad thing, that is not an integer.\n"
That's a bit much, isn't it?

Well, no harm, I changed it a bit.
The class itself is nothing special, but it saves a few lines of code. It's just a small thing to make it easier to write a lot of code.

You should remove the throw(...). Throw specifications have patchy support and often do more harm than good.
The thing won't work with std::wstring, since it will try to write it to cout, not wcout. Frankly, I'd just overload ask() for const std::string& and const std::wstring& and not make it a template at all. const char* and const wchar_t* will be automatically converted to their respective string class forms. This is no efficiency problem, since the next thing you do is wait for user input anyway.
Not to mention, you have a problem with reading in the answer from the narrow stream no matter where you wrote the question to. Not particularly good there.

Perhaps you should just make the whole class a template on the character type and go for the generic versions of everything. You'll need a small trick for switching between cin and wcin and cout and wcout, but that's easy.

Templating the class on the result type, on the other hand, seems like it's not a very good idea. This means you may have to create multiple objects for a series of inputs of different types. This may be fine - but it's something to keep in mind. If you do it this way, you should make the object really represent a single input element and thus move the query-error loop inside the object.

Your input guard lacks uniqueness. INPUT_H is far too common a symbol to use as an input guard. There's few things worse than an input guard conflict in two libraries.

And finally, the Throw template parameter seems to be a relic. You don't use it anywhere.

Well I didn't officially code review it but overall it's a great idea and I'll probably use it. If I do I'll make mention of you in the comments as to where I received it and who wrote it.

Quote:

---

Originally Posted by CornedBee

You should remove the throw(...). Throw specifications have patchy support and often do more harm than good.

---

Okay... I thought I'd add them to allow disambiguation between functions that throws and which does not.

Quote:

---

The thing won't work with std::wstring, since it will try to write it to cout, not wcout. Frankly, I'd just overload ask() for const std::string& and const std::wstring& and not make it a template at all. const char* and const wchar_t* will be automatically converted to their respective string class forms. This is no efficiency problem, since the next thing you do is wait for user input anyway.
Not to mention, you have a problem with reading in the answer from the narrow stream no matter where you wrote the question to. Not particularly good there.

Perhaps you should just make the whole class a template on the character type and go for the generic versions of everything. You'll need a small trick for switching between cin and wcin and cout and wcout, but that's easy.

---

I think I get your point. I should disambiguate char and wchar_t and keep it the same all the way for output and input.
That's not a big deal. It should be easy.

Quote:

---

Templating the class on the result type, on the other hand, seems like it's not a very good idea. This means you may have to create multiple objects for a series of inputs of different types. This may be fine - but it's something to keep in mind. If you do it this way, you should make the object really represent a single input element and thus move the query-error loop inside the object.

---

That was the original idea - to make the object "an" input, but I don't really know if there's more to be done to enforce that. Eventually, the programmer would want the stored value and the easier way to provide that is the get function.
Otherwise if it can be reused, I suppose I could just make get a template function?
Which approach is best in your mind?

Quote:

---

Your input guard lacks uniqueness. INPUT_H is far too common a symbol to use as an input guard. There's few things worse than an input guard conflict in two libraries.

---

Appending a date, such as, INPUT_20081018_H should then work better, I suppose.

Quote:

---

And finally, the Throw template parameter seems to be a relic. You don't use it anywhere.

---

Oh yes, I forgot to remove it.
I toyed with the idea to make the class throwable or not by the use of a template parameter, but then decided on overloading functions, like std::vector does.

Quote:

---

Which approach is best in your mind?

---

Very hard to say. Partially, it's a matter of coding style. Partially, it's a matter of how much you want that class to do.

Quote:

---

I toyed with the idea to make the class throwable or not by the use of a template parameter,

---

Doesn't work properly. I've examined the idea for some projects of mine, and the result was always a mess.

I'm not sure why this fails or how to fix it. Can anyone give a hand?

Code:

---

        template<typename StrType> struct InputTraits // const char*
        {
                static std::basic_ostream<char>& out_stream;
                static std::basic_istream<char>& in_stream;
                typedef std::string StorageType;
        };
        template<typename StrType> std::basic_ostream<char>& InputTraits<StrType>::out_stream = std::cout;
        template<typename StrType> std::basic_istream<char>& InputTraits<StrType>::in_stream = std::cin;

        template<> struct InputTraits<const wchar_t*>
        {
                static std::basic_ostream<wchar_t>& out_stream;
                static std::basic_istream<wchar_t>& in_stream;
                typedef std::wstring StorageType;
        };
        template<> std::basic_ostream<wchar_t>& InputTraits<wchar_t>::out_stream = std::wcout;
        template<> std::basic_istream<wchar_t>& InputTraits<wchar_t>::in_stream = std::wcin;

---

error C2371: 'out_stream' : redefinition; different basic types
error C2086: 'std::basic_ostream<_Elem,_Traits> &Stuff::InputTraits<wchar_t>::out_stream' : redefinition

Mismatch:

Code:

---

template<> struct InputTraits<const wchar_t*>
...
template<> std::basic_ostream<wchar_t>& InputTraits<wchar_t>::out_stream = std::wcout;

---

Call the parameter CharType (or CharT), not StrType, as this is what you're really doing now.

Also, I would have chosen a more descriptive namespace than "Stuff".

Quote:

---

Originally Posted by CornedBee

Mismatch:

Code:

---

template<> struct InputTraits<const wchar_t*>
...
template<> std::basic_ostream<wchar_t>& InputTraits<wchar_t>::out_stream = std::wcout;

---

Call the parameter CharType (or CharT), not StrType, as this is what you're really doing now.

---

Of course.
I need more practice on that one...

Quote:

---

Originally Posted by cpjust

Also, I would have chosen a more descriptive namespace than "Stuff".

---

That's my "Stuff" library ;)

I'll update the source with the newest.
I chose the one object for multiple inputs path, by including a template function for get.

Updated the code a little.
Fixed some bugs, added support for operators << (output) and >> (input).
Currently the operators throws if there is an error (only input, though). I wonder if it's a good idea to make them return a bool? It would make it impossible to chain them, but one wouldn't have to try put try/catch blocks to use them.

Also had trouble breaking out the operators from the class, I just let them be there. It's less messy, too.
You can also see I added a traits header with some traits I've accumulated and used in code (that is, it's common code).

Code:

---

#ifndef TCHAR
        #ifdef UNICODE
                #define TCHAR wchar_t
        #else
                #define TCHAR char
        #endif
#else
        #define TCHAR char
#endif

---

Dangerous. windows.h handle the TCHAR issue with a typedef, not a macro, so your #ifdef won't trigger. (windows.h defines _TCHAR_DEFINED, but I wouldn't rely on it.)
Since a typedef would be scoped anyway, just make it a typedef.

Quote:

---

Originally Posted by Elysia

Currently the operators throws if there is an error (only input, though). I wonder if it's a good idea to make them return a bool? It would make it impossible to chain them, but one wouldn't have to try put try/catch blocks to use them.

---

IIRC, stream insertors and extractors are already convertible to a bool type for the purpose you're thinking of, and that doesn't mitigate the possibility for exceptions. But I have to say I don't think that insertors or extractors are exceptional operations... they usually always attempt to do their work, and return a bad stream if something happened. Unless you're one of those people who opt to throw an exception instead of say program by cases...

Quote:

---

Originally Posted by CornedBee

Code:

---

#ifndef TCHAR
        #ifdef UNICODE
                #define TCHAR wchar_t
        #else
                #define TCHAR char
        #endif
#else
        #define TCHAR char
#endif

---

Dangerous. windows.h handle the TCHAR issue with a typedef, not a macro, so your #ifdef won't trigger. (windows.h defines _TCHAR_DEFINED, but I wouldn't rely on it.)
Since a typedef would be scoped anyway, just make it a typedef.

---

Hmmm.
OK, I changed that a bit.

Now the only problem would be the operators...
Should they cast? Should they not?
Perhaps they should complement the member functions (both throw & non-throw).
And I wouldn't have the slightest clue how to return a "bad stream" in case they fail.

>> And I wouldn't have the slightest clue how to return a "bad stream" in case they fail.

You shouldn't need to "do" anything. Your run-of-the-mill insertor/extractor ...

Code:

---

std::ostream & operator<< (std::ostream & out, const object & o )
{
  out << o.foo();
  out << o.bar();
  return out;
}

---

If foo and bar return plain old data then there is no problem, the state of the stream is determined by the success or failure of the conversion. If foo and bar do not return plain old data then the state of the stream is determined by the result of operator << for the object.

It would be better if you understood that by default streams do not throw.

If you want a specific state to be exceptional you usually call exception( mask ); beforehand.

The problem is that I don't return a stream, but rather the "input object." I'd have to define some "bad" state implementation I guess.
Along with a throw-version and a non-throw version of the operator equivalent member functions.