[date_time] [#1861] Change for the default duration format
Hi, There's a Boost.DateTime ticket: https://svn.boost.org/trac/boost/ticket/1861 The initial problem was that the default handling of the %H format specifier (passing it to strftime) played badly with time durations that could be longer than 24 hours. The resolution was to provide additional %O format specifier that does not have 24 hour restriction. However the original poster proposes to change the default format of time durations from %H:%M:%S%F to %O:%M:%S%F so that formatted durations are always allowed to be longer than 24 hours. I mostly agree with this change, however it is clearly a breaking change that can affect error checking in some cases. What do the community and the maintainer (Jeff?) in particular think about this?
Andrey Semashev wrote On Sunday, May 31, 2009 8:32 AM
There's a Boost.DateTime ticket:
https://svn.boost.org/trac/boost/ticket/1861
The initial problem was that the default handling of the %H format specifier (passing it to strftime) played badly with time durations that could be longer than 24 hours. The resolution was to provide additional %O format specifier that does not have 24 hour restriction.
I suspect that's not common -- other than for the OP, perhaps. Jeff's explanation in the associated list message implies that, too.
However the original poster proposes to change the default format of time durations from %H:%M:%S%F to %O:%M:%S%F so that formatted durations are always allowed to be longer than 24 hours. I mostly agree with this change, however it is clearly a breaking change that can affect error checking in some cases.
Changing the default will create surprises in some cases. I'd prefer to see error checking -- an exception when the valid %H range is violated. That would alert library users to the problem when violated and the exception message could even suggest the need for %O (as should the docs, of course). That would provide useful behavior for those violating the range unknowingly and it would avoid breaking compatibility and, reasonable expectations for users. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Monday 01 June 2009 19:55:04 Stewart, Robert wrote:
The initial problem was that the default handling of the %H format specifier (passing it to strftime) played badly with time durations that could be longer than 24 hours. The resolution was to provide additional %O format specifier that does not have 24 hour restriction.
I suspect that's not common -- other than for the OP, perhaps.
You think so? I don't know how often it is needed to operate longer durations, but IMHO, objectively there is no such restriction for time durations and the library should not artificially impose one.
Jeff's explanation in the associated list message implies that, too.
What message do you mean?
However the original poster proposes to change the default format of time durations from %H:%M:%S%F to %O:%M:%S%F so that formatted durations are always allowed to be longer than 24 hours. I mostly agree with this change, however it is clearly a breaking change that can affect error checking in some cases.
Changing the default will create surprises in some cases. I'd prefer to see error checking -- an exception when the valid %H range is violated. That would alert library users to the problem when violated and the exception message could even suggest the need for %O (as should the docs, of course).
That would provide useful behavior for those violating the range unknowingly and it would avoid breaking compatibility and, reasonable expectations for users.
The change mostly affects formatting. There already is an assert, so users will know if %H is used improperly. The assert is present since 1.38. The question is whether the change to %O as a default is justified enough and how it should be announced better.
Andrey Semashev wrote: On Monday, June 01, 2009 4:09 PM
On Monday 01 June 2009 19:55:04 Stewart, Robert wrote:
The initial problem was that the default handling of the %H format specifier (passing it to strftime) played badly with time durations that could be longer than 24 hours. The resolution was to provide additional %O format specifier that does not have 24 hour restriction.
I suspect that's not common -- other than for the OP, perhaps.
You think so? I don't know how often it is needed to operate longer durations, but IMHO, objectively there is no such restriction for time durations and the library should not artificially impose one.
Given that strftime() uses %OH is for alternative formats, %H is the norm. Consequently, %H seems an unsurprising and useful default for Boost.DateTime, too.
Jeff's explanation in the associated list message implies that, too.
What message do you mean?
This message was mentioned in the issue: http://lists.boost.org/Archives/boost/2007/12/131541.php
However the original poster proposes to change the default format of time durations from %H:%M:%S%F to %O:%M:%S%F so that formatted durations are always allowed to be longer than 24 hours. I mostly agree with this change, however it is clearly a breaking change that can affect error checking in some cases.
Changing the default will create surprises in some cases. I'd prefer to see error checking -- an exception when the valid %H range is violated. That would alert library users to the problem when violated and the exception message could even suggest the need for %O (as should the docs, of course).
That would provide useful behavior for those violating the range unknowingly and it would avoid breaking compatibility and, reasonable expectations for users.
The change mostly affects formatting. There already is an assert, so users will know if %H is used improperly. The assert is present since 1.38.
I question the use of an assert as that's not going to catch errors in runtime values in non-debug builds.
The question is whether the change to %O as a default is justified enough and how it should be announced better.
Using %O in this way is odd, given the strftime() treatment of %O. I haven't looked at the implementation, but I assume that DateTime is interpreting the specifiers itself. If so, there's freedom available, but deviating from strftime() will make things more surprising than necessary. strftime() uses %E and %O as modifiers. Thus, there's precedence for using a capital letter for a modifier, but there aren't many from which to choose ATM: J, K, L, N, and Q. From those, I'd suggest L because %LH can be read as "long hour." I contend that an exception (preceded by an assertion, if desired) for violating %H's range can indicate the need for the %L prefix. Then, the documentation for %H should mention %L's modification of %H's range. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Tuesday 02 June 2009 00:56:53 Stewart, Robert wrote:
I suspect that's not common -- other than for the OP, perhaps.
You think so? I don't know how often it is needed to operate longer durations, but IMHO, objectively there is no such restriction for time durations and the library should not artificially impose one.
Given that strftime() uses %OH is for alternative formats, %H is the norm. Consequently, %H seems an unsurprising and useful default for Boost.DateTime, too.
strftime is not designed to handle time durations in the first place, so its use as an example to follow is questionable. While 24 hour restriction is reasonable for time points, it's not for durations.
The change mostly affects formatting. There already is an assert, so users will know if %H is used improperly. The assert is present since 1.38.
I question the use of an assert as that's not going to catch errors in runtime values in non-debug builds.
I think an assert is enough for various reasons: 1. %H behavior was there for ages, and I don't want to change it even if it means more robust error checking. If it caused problems somewhere before, either the code was fixed or no one cares. The added assert doesn't change that, it only allows to highlight the error in the new code. 2. An exception is not a good choice to highlight the problem, since in most cases it will be swallowed by IO streams. In practice streaming errors (in form of failbit or badbit) are often ignored, and Boost.DateTime is an example. And ios_base::failure usually isn't as descriptive as what is being initially thrown. 3. The caller has to check the arguments. It's the bottom line of both STL and most C functions. Extensive error checking may be provided but it should be optional. An assert fits perfectly.
The question is whether the change to %O as a default is justified enough and how it should be announced better.
Using %O in this way is odd, given the strftime() treatment of %O.
I don't have a C standard on me, but I'm almost sure it doesn't specify %O.
I haven't looked at the implementation, but I assume that DateTime is interpreting the specifiers itself.
No, at least, not all of them. In the end it comes down to call to strftime, last time I checked.
If so, there's freedom available, but deviating from strftime() will make things more surprising than necessary. strftime() uses %E and %O as modifiers. Thus, there's precedence for using a capital letter for a modifier, but there aren't many from which to choose ATM: J, K, L, N, and Q. From those, I'd suggest L because %LH can be read as "long hour."
It's %O already and changing it is not a good idea, IMO. People may rely on it already.
Andrey Semashev wrote On Monday, June 01, 2009 7:01 PM
On Tuesday 02 June 2009 00:56:53 Stewart, Robert wrote:
I missed your message previously, hence the long delay in replying.
I suspect that's not common -- other than for the OP, perhaps.
You think so? I don't know how often it is needed to operate longer durations, but IMHO, objectively there is no such restriction for time durations and the library should not artificially impose one.
Given that strftime() uses %OH is for alternative formats, %H is the norm. Consequently, %H seems an unsurprising and useful default for Boost.DateTime, too.
strftime is not designed to handle time durations in the first place, so its use as an example to follow is questionable. While 24 hour restriction is reasonable for time points, it's not for durations.
strftime() is used as the basis for the conversions and is the obvious source of the meaning of the various format specifiers. Consequently, it remains a viable exemplar of expected behavior. Having said that, I recognize your point about time durations versus time points. If the suggested change only applies to time durations and not to time points, of which I have no idea, I'd be much less concerned.
The change mostly affects formatting. There already is an assert, so users will know if %H is used improperly. The assert is present since 1.38.
I question the use of an assert as that's not going to catch errors in runtime values in non-debug builds.
I think an assert is enough for various reasons:
1. %H behavior was there for ages, and I don't want to change it even if it means more robust error checking. If it caused problems somewhere before, either the code was fixed or no one cares. The added assert doesn't change that, it only allows to highlight the error in the new code.
We're obviously speaking of new code. You haven't justified assertion only.
2. An exception is not a good choice to highlight the problem, since in most cases it will be swallowed by IO streams. In practice streaming errors (in form of failbit or badbit) are often ignored, and Boost.DateTime is an example. And ios_base::failure usually isn't as descriptive as what is being initially thrown.
Sorry, but no. The default is for IOStreams to set the stream state to indicate failures, but that doesn't absorb exceptions from code such as we're discussing and convert exceptions into stream state.
3. The caller has to check the arguments. It's the bottom line of both STL and most C functions. Extensive error checking may be provided but it should be optional. An assert fits perfectly.
A well designed library prevents misuse. That's an argument for changing the default formatting, I grant. It's also an argument for throwing an exception rather than silently truncating if the default formatting isn't changed for other reasons. Yes, folks must use libraries correctly. In C libraries, the functions return error codes which can be ignored. In C++, we have exceptions at our disposal. Furthermore, there are many developers who don't spend much time with debug builds and there are many usage scenarios that can arise untested. The library shouldn't misbehave silently in those cases.
The question is whether the change to %O as a default is justified enough and how it should be announced better.
Using %O in this way is odd, given the strftime() treatment of %O.
I don't have a C standard on me, but I'm almost sure it doesn't specify %O.
Every online strftime manpage I checked, such as the following, describes %O: http://www.opengroup.org/onlinepubs/007908799/xsh/strftime.html
I haven't looked at the implementation, but I assume that DateTime is interpreting the specifiers itself.
No, at least, not all of them. In the end it comes down to call to strftime, last time I checked.
In my mind, that justifies my trying to map to strftime() behavior.
If so, there's freedom available, but deviating from strftime() will make things more surprising than necessary. strftime() uses %E and %O as modifiers. Thus, there's precedence for using a capital letter for a modifier, but there aren't many from which to choose ATM: J, K, L, N, and Q. From those, I'd suggest L because %LH can be read as "long hour."
It's %O already and changing it is not a good idea, IMO. People may rely on it already.
Hmmm. I didn't realize that DateTime already used %O for this purpose. That is a significant argument in favor of using %O, but perhaps my comments in this thread will convince Jeff that he should take another tack. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Monday 15 June 2009 19:42:18 Stewart, Robert wrote: [snip]
Having said that, I recognize your point about time durations versus time points. If the suggested change only applies to time durations and not to time points, of which I have no idea, I'd be much less concerned.
Exactly, I'm speaking of durations only. Time points won't be affected.
2. An exception is not a good choice to highlight the problem, since in most cases it will be swallowed by IO streams. In practice streaming errors (in form of failbit or badbit) are often ignored, and Boost.DateTime is an example. And ios_base::failure usually isn't as descriptive as what is being initially thrown.
Sorry, but no. The default is for IOStreams to set the stream state to indicate failures, but that doesn't absorb exceptions from code such as we're discussing and convert exceptions into stream state.
It does adsorb exceptions, at least on my GCC 4.3.2. But it does pass through original exceptions if exceptions are enabled, but I'm not sure it's guaranteed by the standard. You can play with the attached code sample to test your platform.
3. The caller has to check the arguments. It's the bottom line of both STL and most C functions. Extensive error checking may be provided but it should be optional. An assert fits perfectly.
A well designed library prevents misuse.
Right. But since DateTime IO is based on facets, there isn't much room for design decisions. As for the exception, it does not prevent misuse. It *may* show when the feature *have been* misused.
That's an argument for changing the default formatting, I grant. It's also an argument for throwing an exception rather than silently truncating if the default formatting isn't changed for other reasons. Yes, folks must use libraries correctly. In C libraries, the functions return error codes which can be ignored. In C++, we have exceptions at our disposal. Furthermore, there are many developers who don't spend much time with debug builds and there are many usage scenarios that can arise untested. The library shouldn't misbehave silently in those cases.
We can debate on this. There are people that prefer safety at all costs. There are other people, including me, that don't want to pay for unnecessary run time checks. My position is that there are no guarantees if the code is used incorrectly. If I want safety, I do the proper testing, including debug builds and invalid input to my system to verify that it tackles it well. One advantage of the assert is that it can satisfy both safety and performance needs.
I don't have a C standard on me, but I'm almost sure it doesn't specify %O.
Every online strftime manpage I checked, such as the following, describes %O:
http://www.opengroup.org/onlinepubs/007908799/xsh/strftime.html
I guess, %O was not a good choice in the first place, then. I'm not inclined to change it now, but the final call is after the library maintainer.
I haven't looked at the implementation, but I assume that DateTime is interpreting the specifiers itself.
No, at least, not all of them. In the end it comes down to call to strftime, last time I checked.
In my mind, that justifies my trying to map to strftime() behavior.
I'd consider strftime as an implementation detail.
Andrey Semashev wrote: On Monday, June 15, 2009 5:24 PM
On Monday 15 June 2009 19:42:18 Stewart, Robert wrote:
2. An exception is not a good choice to highlight the problem, since in most cases it will be swallowed by IO streams. In practice streaming errors (in form of failbit or badbit) are often ignored, and Boost.DateTime is an example. And ios_base::failure usually isn't as descriptive as what is being initially thrown.
Sorry, but no. The default is for IOStreams to set the stream state to indicate failures, but that doesn't absorb exceptions from code such as we're discussing and convert exceptions into stream state.
It does adsorb exceptions, at least on my GCC 4.3.2. But it does pass through original exceptions if exceptions are enabled, but I'm not sure it's guaranteed by the standard. You can play with the attached code sample to test your platform.
I went back and reread 27.6.1.2.1. Despite my confident assertion above, there is no distinction made as to the source of exceptions caught. It says, "if an exception is thrown during input then ios::badbit is turned on in *this's error state." I'm sorry for the noise. Throwing an exception, however, does mean that it will propagate from a stream when configured to permit that. Setting badbit is the best that can be done otherwise and seems necessary, even if it is ignored, as it will prevent further operations on the current stream until cleared. That may draw attention to the code using that stream as the result will deviate from the expected. Obviously, badbit can be set without throwing an exception, but the chance for the exception to propagate, with the side effect of setting badbit, means throwing one is worthwhile.
3. The caller has to check the arguments. It's the bottom line of both STL and most C functions. Extensive error checking may be provided but it should be optional. An assert fits perfectly.
A well designed library prevents misuse.
Right. But since DateTime IO is based on facets, there isn't much room for design decisions. As for the exception, it does not prevent misuse. It *may* show when the feature *have been* misused.
Since there's little room for design decisions, why not avail ourselves of all options at our disposal? If throwing an exception doesn't reveal the misuse because exceptions are absorbed, badbit is ignored, and the incorrect result isn't noticed, then there's nothing to be done. However, exceptions may be enabled, badbit may be noticed, and the incorrect result may be noticed, so throwing an exception increases the chance of detecting the misuse.
We can debate on this. There are people that prefer safety at all costs. There are other people, including me, that don't want to pay for unnecessary run time checks. My position is that there are no guarantees if the code is used incorrectly. If I want safety, I do the proper testing, including debug builds and invalid input to my system to verify that it tackles it well. One advantage of the assert is that it can satisfy both safety and performance needs.
We're discussing testing the input against a limit in the midst of all of the normal IOStreams overhead, including multiple virtual function calls. This hardly amounts to performance robbing overhead. An assertion does not satisfy safety concerns completely, and the test to decide whether to throw an exception is no performance drag. Obviously, if the exception is actually thrown, there is real performance effect, but those with your concern for testing well and best performance will have already detected and prevented the condition leading to the exception. The question is whether to change the default formatting for time durations. If done, the question of an exception versus or in addition to an assertion is moot. If not done, for compatibility reasons, then throwing an exception following the assertion is the ideal, best effort way to report the misuse. There's also the issue of whether %O was the right format specifier in the first place, and Jeff Garland must answer that question. (He could opt to accept %OH as a special case equivalent to %O, leaving the latter for compatibility with existing code and documenting the former as the proper means to get the desired formatting. He could even choose to deprecate %O in favor of %OH and remove it some time in the future.) We've beaten on this subject long enough. Jeff must chime in and make the calls. I BCC'd him to try to draw his attention to the discussion. Now we wait. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Stewart, Robert wrote:
Andrey Semashev wrote: On Monday, June 15, 2009 5:24 PM
On Monday 15 June 2009 19:42:18 Stewart, Robert wrote:
2. An exception is not a good choice to highlight the problem, since in most cases it will be swallowed by IO streams. In practice streaming errors (in form of failbit or badbit) are often ignored, and Boost.DateTime is an example. And ios_base::failure usually isn't as descriptive as what is being initially thrown. Sorry, but no. The default is for IOStreams to set the stream state to indicate failures, but that doesn't absorb exceptions from code such as we're discussing and convert exceptions into stream state. It does adsorb exceptions, at least on my GCC 4.3.2. But it does pass through original exceptions if exceptions are enabled, but I'm not sure it's guaranteed by the standard. You can play with the attached code sample to test your platform.
I went back and reread 27.6.1.2.1. Despite my confident assertion above, there is no distinction made as to the source of exceptions caught. It says, "if an exception is thrown during input then ios::badbit is turned on in *this's error state." I'm sorry for the noise.
Throwing an exception, however, does mean that it will propagate from a stream when configured to permit that. Setting badbit is the best that can be done otherwise and seems necessary, even if it is ignored, as it will prevent further operations on the current stream until cleared. That may draw attention to the code using that stream as the result will deviate from the expected. Obviously, badbit can be set without throwing an exception, but the chance for the exception to propagate, with the side effect of setting badbit, means throwing one is worthwhile.
3. The caller has to check the arguments. It's the bottom line of both STL and most C functions. Extensive error checking may be provided but it should be optional. An assert fits perfectly. A well designed library prevents misuse. Right. But since DateTime IO is based on facets, there isn't much room for design decisions. As for the exception, it does not prevent misuse. It *may* show when the feature *have been* misused.
Since there's little room for design decisions, why not avail ourselves of all options at our disposal? If throwing an exception doesn't reveal the misuse because exceptions are absorbed, badbit is ignored, and the incorrect result isn't noticed, then there's nothing to be done. However, exceptions may be enabled, badbit may be noticed, and the incorrect result may be noticed, so throwing an exception increases the chance of detecting the misuse.
General ------- What is so different about time_duration td; cin >> td; from int v; cin >> v; that makes you discuss an alternative error reporting strategy for the time duration input facet? Maybe I'm missing something but it seems to me that the original question was a bit different. I'll try to explain the situation again. There was a bug in the time duration output facet. The facet used strftime to do output for values larger than 24 hours. According to ISO 9899-1999 7.23.3.5 par. 3 this is an undefined behavior. This bug was discussed here: http://lists.boost.org/Archives/boost/2007/12/131541.php And the part of my patch that addressed this bug is already in the library as of 1.39.0. The output for %H is now done without strftime. As %H is used in other format strings besides time duration's one it was decided that a new specifier should be added that will allow input and output of an hours field that is longer than two digits. This new specified is %O. I doubt there is any use for %O in other formats except time duration's one as nor ptime, nor time period have any semantic for an hours field that is larger than 23. On the other hand there is nothing special about a time duration of 23 or 24 hours. There is a distinction between processing of %H in a time duration input facet and a time duration output facet. I'll discuss them separately. The same code is used by input and output facets for all date_time objects. While the format strings may be different for time duration and ptime the %H in these strings will be read identically. %H in an output facet --------------------- The hours value is converted into a string. There is a BOOST_ASSERT that checks that the value is no longer than 2 characters. At the same time a zero is perpended if the value is shorter than 2 characters. Note that there is no check for the value of 23 or 24. Just for the length of 2 characters. A time duration that is 100 hours or longer will assert in a debug build, but will output correctly on a release one. %H in an output facet --------------------- A continuous sequence of digits no longer than 2 is read from a stream. If there is less than 2 digits it is an error. Read digits are converted into a number. Actually it means that the current behavior is wrong with regard to the ptime or time period. But I do not care about them so I leave the fix to someone else. Or will fix it later. See an example at the bottom of the email. For time duration it means that it is OK to read any time duration that is no longer than 99 hour 99 minutes and 99 seconds if you do not change the time duration input format. My proposal ----------- My patch contained a modification to the default *time duration* format string. It does not affect nor ptime, nor time period input format strings. I think that it makes sense to replace %H with %O in the default time duration format to make it possible to input and output arbitrary time durations. There is an opinion that this is "a breaking change" and this part of the patch should be discussed on the mailing list: https://svn.boost.org/trac/boost/ticket/1861#comment:3 Actually I see this as a bug fix. But if someone really thinks that this change may break his or her code it would be nice to hear about it. And probably I will try to find a better fix for this bug. %H in an input facet for a ptime bug ------------------------------------ The following code #include <iostream> #include <sstream> #include <boost/date_time.hpp> using namespace std; using namespace boost::posix_time; int main(void) { time_input_facet* facet = new time_input_facet(); locale loc(locale(locale(), facet)); istringstream ss("2009-Jun-16 28:14:00"); ss.imbue(loc); /* Input using default time format. */ facet->format("%Y-%b-%d %H:%M:%S%F %z"); ptime p; ss >> p; cout << p << endl << "fail is " << boolalpha << ss.fail() << endl; } Gives me this output on MS cl: 2009-Jun-17 04:14:00 fail is false While it would have failed to parse the input value, in my opinion. -- Ilya Bobir
Ilya Bobir wrote:
General -------
What is so different about
time_duration td; cin >> td;
from
int v; cin >> v;
that makes you discuss an alternative error reporting strategy for the time duration input facet?
Because apparently not everyone is happy with the current strategy.
%H in an output facet --------------------- The hours value is converted into a string. There is a BOOST_ASSERT that checks that the value is no longer than 2 characters. At the same time a zero is perpended if the value is shorter than 2 characters.
Note that there is no check for the value of 23 or 24. Just for the length of 2 characters.
Technically, this check is intended to ensure that the formatted string will be parsed back with the %H specifier. That will hold if hours string is no longer than 2 characters.
%H in an output facet --------------------- A continuous sequence of digits no longer than 2 is read from a stream. If there is less than 2 digits it is an error. Read digits are converted into a number. Actually it means that the current behavior is wrong with regard to the ptime or time period.
What exactly is wrong?
%H in an input facet for a ptime bug ------------------------------------ The following code
[snip]
Gives me this output on MS cl:
2009-Jun-17 04:14:00 fail is false
While it would have failed to parse the input value, in my opinion.
I believe it's not a bug. The read date is normalized correctly.
Andrey Semashev wrote:
Ilya Bobir wrote:
General -------
What is so different about
time_duration td; cin >> td;
from
int v; cin >> v;
that makes you discuss an alternative error reporting strategy for the time duration input facet?
Because apparently not everyone is happy with the current strategy.
If the time period input and/or output facet will do error reporting in a non-standard way would not it be confusing? I thought that if I'm using a type with a "standard" input/output stream then it should behave in a "standard" way. Why would I want to go into time period error reporting details? What make it so different that as a user I really should spent my time reading the docs and why should I break my "normal" usage patterns for the time period or any other data time object? This is what I'm missing. What is so special about time periods? :) If there is something unsatisfactory with the error reporting for the input/output facets in general. Then I think it should be a separate thread. Just to make it prominent that this question is not related to acceptance of my patch =)
%H in an output facet --------------------- The hours value is converted into a string. There is a BOOST_ASSERT that checks that the value is no longer than 2 characters. At the same time a zero is perpended if the value is shorter than 2 characters.
Note that there is no check for the value of 23 or 24. Just for the length of 2 characters.
Technically, this check is intended to ensure that the formatted string will be parsed back with the %H specifier. That will hold if hours string is no longer than 2 characters.
%H in an output facet --------------------- A continuous sequence of digits no longer than 2 is read from a stream. If there is less than 2 digits it is an error. Read digits are converted into a number. Actually it means that the current behavior is wrong with regard to the ptime or time period.
What exactly is wrong?
%H in an input facet for a ptime bug ------------------------------------ The following code
[snip]
Gives me this output on MS cl:
2009-Jun-17 04:14:00 fail is false
While it would have failed to parse the input value, in my opinion.
I believe it's not a bug. The read date is normalized correctly.
If we do normalization then why don't we allow for 3, 4 or 5 digits in the hours field? Certainly there could be more than one opinion on this subject but I think that the ptime input facet should fail if it reads value of 27 in the hours field. This is something that can not be generated by a ptime output facet and is very suspicious. What will you think if your digital clock will show you that it is 27:16? I doubt that you will think that it is 03:16 of the next day :) In other words I think that range [0, 24] for the hours field is part of the ptime syntax. And input facet should fail to parse a string that is syntactically incorrect - as the one used in my example. I have not read the "ISO 8601", but a shallow examination seems to show that the standard does not allow anything except [0, 24] for the field and the value of 24 is allowed only under special circumstances. As there are references to this standard in the documentation it adds confusion if the library behaves differently from what is written in the standard.
On Wednesday 17 June 2009 14:40:20 Ilya Bobir wrote:
Andrey Semashev wrote:
Because apparently not everyone is happy with the current strategy.
If the time period input and/or output facet will do error reporting in a non-standard way would not it be confusing? I thought that if I'm using a type with a "standard" input/output stream then it should behave in a "standard" way. Why would I want to go into time period error reporting details? What make it so different that as a user I really should spent my time reading the docs and why should I break my "normal" usage patterns for the time period or any other data time object? This is what I'm missing. What is so special about time periods? :)
I'm not sure what you're advocating for. What error reporting strategy do you call "standard"? In the discussed matter an assert is used - is it "standard" or not, in your terms?
While it would have failed to parse the input value, in my opinion.
I believe it's not a bug. The read date is normalized correctly.
If we do normalization then why don't we allow for 3, 4 or 5 digits in the hours field?
I think, I've said it somewhere. %H is bound exactly to 2 digits to support format specifiers without delimiters, such as %H%M%S. It is irrelevant to 24- hour limit.
Certainly there could be more than one opinion on this subject but I think that the ptime input facet should fail if it reads value of 27 in the hours field. This is something that can not be generated by a ptime output facet and is very suspicious. What will you think if your digital clock will show you that it is 27:16? I doubt that you will think that it is 03:16 of the next day :)
I would think so, actually. I admit that 27:16 is not a usual time representation, but nevertheless I think that normalization may be useful and is a valid behavior.
In other words I think that range [0, 24] for the hours field is part of the ptime syntax. And input facet should fail to parse a string that is syntactically incorrect - as the one used in my example.
It is syntactically correct.
I have not read the "ISO 8601", but a shallow examination seems to show that the standard does not allow anything except [0, 24] for the field and the value of 24 is allowed only under special circumstances. As there are references to this standard in the documentation it adds confusion if the library behaves differently from what is written in the standard.
In any case, I think, we both in no position to settle this argument. I'd prefer to see what the maintainer thinks of it.
Andrey Semashev wrote:
On Wednesday 17 June 2009 14:40:20 Ilya Bobir wrote:
Andrey Semashev wrote:
Because apparently not everyone is happy with the current strategy. If the time period input and/or output facet will do error reporting in a non-standard way would not it be confusing? I thought that if I'm using a type with a "standard" input/output stream then it should behave in a "standard" way. Why would I want to go into time period error reporting details? What make it so different that as a user I really should spent my time reading the docs and why should I break my "normal" usage patterns for the time period or any other data time object? This is what I'm missing. What is so special about time periods? :)
I'm not sure what you're advocating for. What error reporting strategy do you call "standard"? In the discussed matter an assert is used - is it "standard" or not, in your terms?
I thought that there is a "standard" error reporting strategy supported by the C++ standard. There is an ios_base::iostate class (ISO 14882-2003 27.4.2.1.3) and a number of functions in the basic_ios class (ISO 14882-2003 27.4.4.3) that deal with iostate objects. Paragraph 2 of ISO 14882-2003 22.2 says the following:
The put() members make no provision for error reporting. (Any failures of the OutputIterator argument must be extracted from the returned iterator.) The get() members take an ios_base::iostate& argument whose value they ignore, but set to ios_base::failbit in case of a parse error.
It means that all default input facets set failbit in case of an error. Unless I'm missing something this is an offtopic on this thread. The title of this thread is "Change for the default duration format". And that question is on how an error should be reported by an input facet. This is why I'm asking what is so special about time periods? I mean why these quite distinct things are been discussed in the same thread?
Ilya Bobir wrote: On Wednesday, June 17, 2009 6:40 AM
Andrey Semashev wrote:
Ilya Bobir wrote:
What is so different about
time_duration td; cin >> td;
from
int v; cin >> v;
that makes you discuss an alternative error reporting strategy for the time duration input facet?
Because apparently not everyone is happy with the current strategy.
If the time period input and/or output facet will do error reporting in a non-standard way would not it be confusing?
What is nonstandard about the input facet throwing an exception if the input isn't valid? _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Andrey Semashev wrote:
What exactly is wrong?
Streaming of a time duration is broken; IMHO the snippet below shouldn't be asserted on. To make things consistent, alternatives could be to impose a limit of 100 hours in the concept of a time duration, to introduce a limited_time_duration concept, to follow ISO 8601, or to allow more than 99 hours. time_duration a = hours( 10 ); // OK std::cout << a << std::endl; // OK time_duration b = hours( 100 ); // OK std::cout << b << std::endl; // Assertion failure Kind regards, Rutger ter Borg
Rutger ter Borg wrote:
Andrey Semashev wrote:
What exactly is wrong?
Streaming of a time duration is broken; IMHO the snippet below shouldn't be asserted on. To make things consistent, alternatives could be to impose a limit of 100 hours in the concept of a time duration, to introduce a limited_time_duration concept, to follow ISO 8601, or to allow more than 99 hours.
time_duration a = hours( 10 ); // OK std::cout << a << std::endl; // OK
time_duration b = hours( 100 ); // OK std::cout << b << std::endl; // Assertion failure
The assertion is there because the default time duration format is "%H:%M:%S". This is exactly what I'm proposing to change. I think it should be "%O:%M:%S" thus imposing no restrictions on the hours field of a time duration. Actually, I can not find a notion of a limited time duration in ISO 8601-2004. Instead the third paragraph of section 4.4.3.2 says the following:
In these representations the maximum number of digits in a component needs to be agreed by the partners in information interchange.
Suggesting that time durations may have variable number of digits for all fields.
participants (4)
-
Andrey Semashev -
Ilya Bobir -
Rutger ter Borg -
Stewart, Robert