argparse icon indicating copy to clipboard operation
argparse copied to clipboard

Better ways to deal with data type?

Open lingerer opened this issue 3 years ago • 2 comments

After an agumentParse parse successful,We can get the value:


 template <typename T>
  auto get() const
      -> std::conditional_t<details::IsContainer<T>, T, const T &> {
    if (!m_values.empty()) {
      if constexpr (details::IsContainer<T>) {
        return any_cast_container<T>(m_values);
      } else {
        return *std::any_cast<T>(&m_values.front());
      }
    }
    if (m_default_value.has_value()) {
      return *std::any_cast<T>(&m_default_value);
    }
    if constexpr (details::IsContainer<T>) {
      if (!m_accepts_optional_like_value) {
        return any_cast_container<T>(m_values);
      }
    }

    throw std::logic_error("No value provided for '" + m_names.back() + "'.");
  }

The actual values are stored in m_values(vector) and by m_default_value (when m_default_value is empty) In my practices , it may result some inconsistent result. It's a long story,I will try to make it clear through my path. 1.nullptr error If you just begin to try argparse,you will glad to see this will work test --arg 12

test.add_argument("--arg");
test.parse_args(argc, argv);
auto s_arg = test.get<string>("--arg");
std::cout<<"arg:"<<s_arg<<std::endl;

but this will result nullptr error

test.add_argument("--arg");
test.parse_args(argc, argv);
auto s_arg = test.get<int>("--arg");
std::cout<<"arg:"<<s_arg<<std::endl;

get will return a nullptr exception cause by return *std::any_cast<T>(&m_values.front());,the exception will not be catch . So my first suggestion is catch the any_cast exception before return,if any_cast fail just throw the std::logic_error.

2.value type problem? Then I dig more

test.add_argument("--arg").default_value(1.1);
test.parse_args(argc, argv);
auto f_arg = test.get<float>("--arg");
auto d_arg = test.get<double>("--arg");

auto f_arg = test.get<float>("--arg"); will result nullptr and auto d_arg = test.get<double>("--arg"); won't. 1.1 is treat as double so that's why get<float> will result nullptr exception. test.add_argument("--arg").default_value(float(1.1)); will make get<float> work but get<double> failed,so that's all about std::any type. But... what if this code test.add_argument("--arg").default_value(1.1).scan<'f',float>(); default value will be treat as double and scanner will convert to float.It will result great confusion in practices. So my second suggestion is force a type compare between default value and scanner or provide a default scanner when provide default_value. This issue is m_default_value m_values may contain different data type.

3.More thoughts When I found almost all the pains is due to how to cast string argument to std::any and then cast back to datatype and the possible data type different between m_default_value and m_values.Why not provide a way just store the string argument? Is it possible or it just cost too much for common use?

lingerer avatar Oct 19 '22 08:10 lingerer

1 You surprised me with this one because failures on that line used to throw std::bad_any_cast. I believe that is still the proper exception. There was a fix for this mixed in with other changes that were not accepted in PR #199. We should try to resurrect those two lines.

2 My first reaction is: "don't do that". Your examples act that way because the chosen values are misleading the compiler. 1.1 is a double, 1.1f is a float.

IMHO, developers should be aware of their types. Yes, when I first began using argparse, I caused similar problems for myself. But, these errors told me to be more careful about the value type going in and coming out.

3 Storing the string and delaying the conversion means the developer's error may not be shown until very late.

skrobinson avatar Oct 27 '22 15:10 skrobinson

2 The main suggestion is the default value should apply scanner as well,not only value. I can't offer a code patch cause I'm a old dog only can read but not able to produce a template code.

lingerer avatar Oct 28 '22 04:10 lingerer