Thread: Input class project (again)

My thoughts are something like:

Code:

template<typename DataT> bool inline InternalValidator(const DataT& Data) { return true; }

template< typename DataT, typename StringT, typename Validator = &InternalValidator<DataT>, typename InputStream = std::basic_istream<StringT> >
class Input
{
public:
	Input(const StringT& Question, const StringT& ErrorType, const StringT& ErrorValidation, Validator ValidatorFnc);
	Input(const StringT& Question, const StringT& ErrorType);
	Input(const StringT& Question);
	Input();

	Input& operator >> (DataT& rhs);

protected:
	virtual bool Convert(const StringT& ConvertFrom, const DataT& ConvertTo);

	StringT m_Data;
};

Because it avoids several overloads of the same code and allows for polymorphism.
Potentially, the constructor asks and fetches the answer, as well as validated and loops until the answer is valid.
The answer is then converted and extracted through the >> operator.

First experimental implementation (see attached file).
Thoughts? Comments?
It seems difficult to implement this as functions unless there are many, and even then, there are problem with default template parameters.

It seems difficult to implement this as functions unless there are many

I have no idea what you're talking about. My suggested code can do everything yours can do (you can overload read() on the data type if you need special reading and converting), is shorter, simpler, and easier and more concise to use. (Overloads for not having a validator are just three one-liners.)

Design issues aside, here are the actual problems with your code:
1) You're calling virtual functions from the constructor. They won't behave polymorphically. I.e., try deriving a class from Input where you override ReadData or Convert. Those versions won't actually be called. This is probably the biggest issue, since it completely negates any advantage the class might have had.
2) Your operator >> is unintuitive. When I do cin >> x >> y I expect to get two results. When I do input >> x >> y, both variables will get the same value.
3) You have a template parameter InputStream, you don't actually use it. Also, std::basic_istream<StringT> might be allowed by the compiler, but it will definitely not produce anything usable when you actually use it. In the end, you just use std::cin and std::cout unconditionally.
4) Does the default parameter to the constructor even work? This doesn't, so I'm inclined to think that your code doesn't, either:

Code:

#include <iostream>

template <typename T>
void f(T t = 100)
{
  std::cout << t;
}

int main()
{
  f();
}

The compiler cannot deduce the template argument from the type of a default parameter.
5) I don't think the distinction between a failure to convert the string to the target type and a validation failure is all that useful. But if you have it, then at least you should allow substitution of the value into the validation failure string.

It seems difficult to implement this as functions unless there are many

That was probably poorly worded, but I remembered its meaning now. I was trying to express that it might be difficult to implement as functions, unless I would create and overload lots of free functions instead of like wrapped in one class.
But you are right - I will try to break this into free functions.

As for the rest of the feedback...
1 & 2) Yes, my mistake. Although by just storing the parameters in the constructor and putting all the code inside the operator >>, I might eliminate those two. I think. Good to remember.
3) It's likely a relic... it was supposed to be used. At least later. But in this experimental implementation, I did not substitute cout, cin, etc.
4) For some reason VS just complains "2>error C2660: 'Input<DataT>::Input' : function does not take 3 arguments". Now what I had expected...
5) That is a good idea.

Anyway, this was sort of what I wanted to encapsulate in functions or a class. Now the problem is just to discover if it's feasible to make as free functions.

UPDATE:
I managed to implement them as free functions.
The only problem I see now (that I did not see before), is some more template trickery to select a proper string type to store data in. I'm considering std::basic_string<T>, but it would require me to fetch the base-type of the string template parameter.
I put a comment there. Before the std::getline function.
Hopefully this is a good approach.

You wouldn't need the const char* overloads if you followed my code and templated on the character type. Do you really use any string type besides std::string that's interface-compatible? Remember that part of "interface-compatible" here includes being able to use std::getline, which limits you to std::basic_string anyway.

Your direct usage of std::cin is problematic for you, as you've discovered. You should follow my lead again and inject the input and output streams at the outermost interface level.

However, if you just want the character type underlying a std::basic_string instantiation, it's the nested value_type typedef.

On the nitpicking side, why is the argument to ValidateRange a reference? It's not like the function is generic.

Originally Posted by CornedBee

You wouldn't need the const char* overloads if you followed my code and templated on the character type. Do you really use any string type besides std::string that's interface-compatible? Remember that part of "interface-compatible" here includes being able to use std::getline, which limits you to std::basic_string anyway.

Your code relies only on std::basic_string<T>. I have no intention of limiting myself in that way. Especially since I do have a custom string type lying around unfinished.

Your direct usage of std::cin is problematic for you, as you've discovered. You should follow my lead again and inject the input and output streams at the outermost interface level.

They will be. Along with std::getline. That is why I need the flexibility to change the behavior of the functions, which is mostly why functions are so problematic.

However, if you just want the character type underlying a std::basic_string instantiation, it's the nested value_type typedef.

So how many std types actually have this value_type typedef?

On the nitpicking side, why is the argument to ValidateRange a reference? It's not like the function is generic.

'Fraid I don't understand that one.

Originally Posted by Elysia

So how many std types actually have this value_type typedef?

Besides std::basic_string, all the standard containers.

Originally Posted by Elysia

Your code relies only on std::basic_string<T>. I have no intention of limiting myself in that way. Especially since I do have a custom string type lying around unfinished.

*shrug*
If you think that's necessary.
At least template on the char type so that wide string literals can be used.

They will be. Along with std::getline. That is why I need the flexibility to change the behavior of the functions, which is mostly why functions are so problematic.

That's what overloading is for.

So how many std types actually have this value_type typedef?

All that comply with the Container concept, i.e. std::basic_string, std::vector, ...

'Fraid I don't understand that one.

You have

Code:

bool ValidateRange(const int& Input)

Why take Input by reference?

Originally Posted by CornedBee

*shrug*
If you think that's necessary.

Part of the problem with the standard containers is that they do no lend well to deriving. So that makes pretty much any "other" string class stand-alone and not derived from std::string. Silly, really, and causes a lot of problems.
But for strings, perhaps const char* or const wchar_t* might suffice, seeing as I would see a lot of string classes implement such. But then again, it does reduce the flexibility a bit...

At least template on the char type so that wide string literals can be used.

Wide strings are to be supported, of course.

That's what overloading is for.

But how would you overload the ReadData function without overloading and re-implementing the rest of the functions that call it?

You have

Code:

bool ValidateRange(const int& Input)

Why take Input by reference?

Who says input is limited to native types?

Looks quite OK to me.

Code:

	template<typename DataT>
	void Read(DataT& Out, const char* Question, const char* ErrorType, const char* ErrorValidation);

This seems unneeded since const char* are covered by the templated version.

Code:

bool ValidateRange(const int& Input);

This would be more useful as a function object so you can write something like:

Code:

Input::Read(
    "Enter an integer: ", 
    "That is not a valid integer. Try again.", 
    "The integer must be bigger than 10 and lower than 20.", 
    ValidateRange(10, 20)
);

However, notice how the values 10 and 20 are duplicated in two places and how they could go out of sync easily...

Originally Posted by anon

This seems unneeded since const char* are covered by the templated version.

VS barks that the template type is ambiguous if I don't add the overload.
It identifies them as char[n], and as they are different lengths, it does not know the type is char[x] or char[y].
I do not wish to explicitly tell it to use const T*.

This would be more useful as a function object so you can write something like:

Code:

Input::Read(
    "Enter an integer: ", 
    "That is not a valid integer. Try again.", 
    "The integer must be bigger than 10 and lower than 20.", 
    ValidateRange(10, 20)
);

However, notice how the values 10 and 20 are duplicated in two places and how they could go out of sync easily...

Hmmm. I think I had something like that planned... predefined validation functions. The one I did was a test only...
But perhaps the validation part can be wrapped inside a struct or class...
It would add a small overhead, but it could auto-construct an error in a way, if a custom one was not provided.
Something to think about.

Code:

class RangeValidation
{
public:
    RangeValidation(int Min, int Max)
    {
        ErrorMsg = "Bla bla bla " + boost::lexical_cast<std::string>(Min) // ... ;
    }

    FunctionType Validator;
    std::string ErrorMsg;
}

Originally Posted by Elysia

Part of the problem with the standard containers is that they do no lend well to deriving.

I don't think this is a problem, but then I'm much less likely to choose derivation as the method of choice for any given problem than you.

So that makes pretty much any "other" string class stand-alone and not derived from std::string.

The only "other" string classes that make sense are those that use a different storage method, so they couldn't derive anyway.

But how would you overload the ReadData function without overloading and re-implementing the rest of the functions that call it?

Argument-dependent lookup.

Who says input is limited to native types?

The type signature. It says "int" right there.

Another technical problem trying to overcome the repetion of values 10 and 20

Code:

template <class T>
class ValidateRangeT
{
    T lower, upper;
public:
    ValidateRangeT(const T& lower, const T& upper): lower(lower), upper(upper) {}
    bool operator () (const T& value) const
    {
        return value > lower && value < upper;
    }
};

template <class T>
ValidateRangeT<T> ValidateRange(const T& low, const T& high)
{
    return ValidateRangeT<T>(low, high);
}


int main()
{
	int x;
	int low = 10;
	int high = 20;
	Input::Read<int, std::string>(
	    x,
            "Enter an integer: ",
            "That is not a valid integer. Try again.",
            boost::str(boost::format("The integer must be bigger than %d and lower than %d.") % low % high),
            ValidateRange(low, high)
        );
	std::cout << "You entered: " << x << std::endl;
}

I have to go to some lengths because you assume that all the messages will be of the same type. (This may also be the reason why you "needed" the const char* version.)

Ideally I should be able to write the following (since boost::format overloads operator<<)

Code:

int main()
{
	int x;
	int low = 10;
	int high = 20;
	Input::Read(
	    x,
            "Enter an integer: ",
            "That is not a valid integer. Try again.",
            boost::format("The integer must be bigger than %d and lower than %d.") % low % high,
            ValidateRange(low, high)
        );
	std::cout << "You entered: " << x << std::endl;
}

Wouldn't boost::lexical_cast spot that first, though?