Poco::DateTime uses assertions for validation

[kimgr]

We're unit-testing some code based on Poco::DateTime.

When we throw invalid dates at our code, this is printed to the console:

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Assertion violation: month >= 1 && month <= 12 [in file "src/DateTime.cpp", line 67]
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

It turns out because DateTime uses poco_assert for validation, there doesn't seem to be a clean way to validate date times without double-checking them.

Would you consider patches to throw a documented exception type instead? Any ideas on which type is preferable? Just a Poco::Exception?

[obiltschnig]

Yes. Throwing Poco::InvalidArgumentException would be appropriate.

[kimgr]

Thanks! Sorry I never got around to this, something more urgent came up.

Meanwhile, I read about narrow vs wide contracts [1] and started to think that maybe the DateTime constructor is better off with a narrow contract with undefined behavior in the presence of obviously bad data.

That said, Poco strikes me as trying to avoid undefined behavior for the most part, so an exception seems more in style. Let me know what you prefer.

[1] http://stackoverflow.com/a/16734278/96963

[PureAbstract]

For what little it's worth, I'd prefer that the constructor validates and throws on error.
Other alternatives might be have an 'IsValid()' member that could be called by those of us who care about avoiding undefined behaviour - another option might be for callers to pass an error_code by reference (as in std::filesystem).

Whichever way it's done, in a library like Poco, there should be some way for the caller to verify that DateTime is doing what they expect it to do.

[SmileGobo]

The bug is still in 1.13.3