handling types

[peterritter]

Hi josua
I've just started to test your API. Are you receptive to new ideas? If not it's ok, just ignore me. But if you are, here are some ideas:

(1)

O.get().u32

I don't really like the look of the above. Why not simply allows access like this:

O.get<unsigned>()
O.get<double>()
O.get<std::string>()

You have defined a bunch of types yourself, but they pretty much map straight back to c++ types. Why not use the C++ types directly? Use your own types internally only. If the conversion fails, throw an exception. Maybe even better:

if(parser.available("optimization")){
        int o = parser.get<int>("optimization");
}

I think it worth thinking about this. I'm sure its not entirely straight forward, especially in light of interpreting empty string values/ 'null-strings'. I had many such situations in my own code recently, and I usually found that a simple solution was usually possible. I'm happy to discuss.

(2)
Also, personally I don't like the:

auto&& O = parser["optimization"];

I'm not even sure what the auto&& is really for. Is a simple reference not enough?

Thanks, Peter

[peterritter]

hi

For declaring options, one way of doing it would be:

parser.option<int>("optimization",'O').help("some help text");

Thanks, Peter

[Fytch]

Hello

Thanks a lot for your feedback. I appreciate every suggestion and criticism.

(1.1)

O.get<unsigned>()

Well, O.get().u32 is actually both shorter to type and more expressive (it tells you the exact amount of memory and the value range), so I vastly prefer it. Do you not agree?

(1.2)

if(parser.available("optimization")){
int o = parser.get<int>("optimization");
}

You shouldn't look up the string "optimization" every time you use it. Every instance of "optimization" means another lookup in the internal hashmap. That's why I saved references to the option in my examples and that's how you should do it too. Then your code becomes much simpler:

if(optimization.available()){
        auto o = optimization.get().i32;
}

(2)
auto& may be sufficient for certain cases but auto&& is more generic. It also binds rvalue-references and adopts CV-qualifiers. If you're not familiar with C++11's new auto&&, here's a link: https://stackoverflow.com/questions/13230480/what-does-auto-tell-us

(3)

parser.option<int>("optimization",'O').help("some help text");

The problem with this is that I could not store the values in a union anymore. I would need to implement a value class with complete type-erasure which would require 1. RTTI enabled and 2. typeid, which is generally slow. And on the other hand, what is gained by this? A slightly different syntax? I don't consider it worth it.

Cheers

[peterritter]

hi
I'm glad you appreciate my feedback.

(1)
If there is any confusion about size a user can easily type

O.get<uint32_t>()

functions with a signature like get() and as() are used so widely and are so useful that I don't see why something new needs to be invented.

(2)
You know, some libraries need performance and memory optimization, and others need convenience. Parsing program options falls into the latter category. Nobody is going to worry about a microsecond when parsing program options. Nobody is going to want to upgrade to the latest compiler just to use it either. In large projects like mine you can't just upgrade compilers and IDE for the sake of using one library. We'll get around to that eventually, but not immediately.

Ok, thanks for your consideration!
Best Regards, Peter

[Fytch]

Hello

(1)
I agree, get templates are pretty commonplace. I'll have a look into that, if you're still interested. Would you mind testing it once I've implemented it? I envisioned keeping the .type(i32) syntax for setting the options' types but offering an additional get template that automatically converts the type if possible.

(2)
Fair enough. Unfortunately, I can't change the value class to store an arbitrary type at the given moment, because that would basically require me to rewrite the whole library. Everything is built upon the assumption that only the given 8 value_types are possible. And rewriting everything would mean shedding backwards compatibility completely. But I will definitely keep that in mind for version v2.0! Someone has actually suggested something similar because they wanted to specialize get for their own class type, so I guess you're in the majority ;)

As to the point with the compiler: I see what you mean but if you're alluding to the other issue, that Visual Studio 2015 won't compile my library, that is unfortunately out of my scope. It is quite obviously a compiler bug. My code is C++11-compliant, so there's nothing to "fix" on my end. However, I have figured out a workaround if your standard library supports C++14 and I'm going to post it in the other issue.

Thanks for the cooperation.
Cheers

[peterritter]

hi

You don't have to support 'arbitrary' types! The ones you are supporting are quite sufficient. You can easily restrict to just the types you are supporting. I have a 'variant' class that I use in all my projects. It is an extremely useful class. In fact it could easily be the basis for a library like yours as it takes care of all type conversions. It also has an internal type enum like you are using, but I do have get() and as() members. I restrict the allowed template argument types by doing this:

	template<typename T> T get(void) const {
		static_assert(std::is_same<T, int64_t>::value ||
			std::is_same<T, double>::value ||
			std::is_same<T, float>::value ||
			std::is_same<T, int64_t>::value ||
			std::is_same<T, uint32_t>::value ||
			std::is_same<T, int32_t>::value ||
			std::is_same<T, bool>::value ||
			std::is_same<T, std::string>::value ||
			std::is_same<T, tsa::date>::value ||
			std::is_same<T, tsa::date_time>::value ||
			std::is_same<T, tsa::duration>::value
			, "variant::get<T>() template argument not supported");
	}

I'll test your compiler fix tomorrow. I'm near London, its late here :-)
Peter

[peterritter]

Hi
I just had another thought. Why does the user have to declare the 'type' of an option at all? To start with, every option value is a string. Its only when the user does .get<double>() that you need to parse the string to see if it converts cleanly to a 'double'. The only reason why users would have to declare a type for an option is if they expected the string to be error checked at the time parser::parse() is called. That is what I would do. If the user does pass a type, then you use it to error check the input immediately. If the user does not give a type, then you only throw an optional error at the time the user callsget<T>()sometimes later.

Food for though! Sorry to bombard you with all these ideas.

Peter