[chromium-dev] Re: Overloading operator<< for TimeDelta

[Evan Martin]

We have that operator defined for a bunch of random types (gfx::Rect
comes to mind) so I think if there's a compilation overheard it should
be fixed in other places as well.

On Mon, Aug 24, 2009 at 10:26 AM, Andrew Scherkus wrote:
> I'm with Matt on this one but if there are serious objections I'll let this
> die
>
> http://codereview.chromium.org/173234/show
>
> On Fri, Aug 21, 2009 at 10:27 AM, Matt Perry 
> wrote:
>>
>> Defining operator<< is fine. Other types do this:
>>
>> http://www.google.com/codesearch?as_q=operator<<&btnG=Search+Code&hl=en&as_lang=&as_license_restrict=i&as_license=&as_package=src.chromium.org/svn/trunk/src&as_filename=\.h&as_case=
>>
>> string16 is a good example.
>> I say just use iosfwd and add the operator.
>> On Thu, Aug 20, 2009 at 10:10 PM, Jim Roskind  wrote:
>>>
>>> Looking at the example you gavehow about:
>>> EXPECT_EQ(kExpected.InMilliseconds(), foo.InMilliseconds());
>>> Is that really that painful to write?
>>> ...and you could get all the microseconds to compare if you wanted to via
>>> ...InMicroseconds().
>>> I suspect you don't really want absolute comparisons at the microsecond
>>> level, and more likely you'd want something like;
>>> EXPECT_LT((kEpected - foo).InMilliseconds(), 20).
>>> ...but if you really wanted the example you cited, the first line seems
>>> relatively short.
>>> Jim
>>> On Thu, Aug 20, 2009 at 7:13 PM, Andrew Scherkus 
>>> wrote:

 I know microseconds aren't a very user-friendly format, but for unit
 tests and DCHECKs I'm more interested in whether the assertion is simply
 true.
 Perhaps I'm lazy but I'd prefer:
 EXPECT_EQ(kExpected, foo);
 error: Value of: foo
   Actual: 2100
 Expected: kExpected
 Which is: 2200
 ...over:
 EXPECT_TRUE(kExpected == foo) << "Some message about " <<
 kExpected.InSecondsF() << " and " << foo.InSecondsF();
 error: Value of: kExpected == foo
   Actual: false
 Expected: true
 Some message about 21.0 and 22.0
 Guaranteed I won't write that message every time and then I end up with
 a simple true/false dump instead of the erroneous values.
 On Thu, Aug 20, 2009 at 6:49 PM, Matt Perry 
 wrote:
>
> Andrew wants to be able to do:
>   DCHECK_EQ(expected_time_delta, time_delta);
> This can't be done without operator<< support.
>
> On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:
>>
>> +1 for Peter's suggestion.
>> TimeDelta has an internal accuracy of microseconds.  What
>> resolution/scaling do you want to print in a check?  Sometimes it is
>> minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
>> microseconds :-/.
>> Explicit conversion as suggested doesn't seem that painful IMO.
>> Jim
>>
>> On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting 
>> wrote:
>>>
>>> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus
>>>  wrote:

 Any opposition to globally declaring an operator<< ostream overload
 for TimeDelta in base/time.h?
>>>
>>> This will pull the stream headers into all files that use time.h.  Is
>>> that going to bloat any code or cost compile time?
>>> Is there another easy solution like doing DCHECK() << "TimeDelta was:
>>> " << myTimeDelta.asInt64OrWhatever()?
>>> PK
>>>
>>
>>
>>
>

>>>
>>
>
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Andrew Scherkus]

I'm with Matt on this one but if there are serious objections I'll let this
die

http://codereview.chromium.org/173234/show

On Fri, Aug 21, 2009 at 10:27 AM, Matt Perry wrote:

> Defining operator<< is fine. Other types do this:
> http://www.google.com/codesearch?as_q=operator<<&btnG=Search+Code&hl=en&as_lang=&as_license_restrict=i&as_license=&as_package=src.chromium.org/svn/trunk/src&as_filename=\.h&as_case=
>
> string16 is a good example.
>
> 
> I say just use iosfwd and add the operator.
>
> On Thu, Aug 20, 2009 at 10:10 PM, Jim Roskind  wrote:
>
>> Looking at the example you gavehow about:
>> EXPECT_EQ(kExpected.InMilliseconds(), foo.InMilliseconds());
>>
>> Is that really that painful to write?
>>
>> ...and you could get all the microseconds to compare if you wanted to via
>> ...InMicroseconds().
>>
>> I suspect you don't really want absolute comparisons at the microsecond
>> level, and more likely you'd want something like;
>>
>> EXPECT_LT((kEpected - foo).InMilliseconds(), 20).
>>
>> ...but if you really wanted the example you cited, the first line seems
>> relatively short.
>>
>> Jim
>>
>> On Thu, Aug 20, 2009 at 7:13 PM, Andrew Scherkus 
>> wrote:
>>
>>> I know microseconds aren't a very user-friendly format, but for unit
>>> tests and DCHECKs I'm more interested in whether the assertion is simply
>>> true.
>>>
>>> Perhaps I'm lazy but I'd prefer:
>>> EXPECT_EQ(kExpected, foo);
>>> error: Value of: foo
>>>   Actual: 2100
>>> Expected: kExpected
>>> Which is: 2200
>>>
>>> ...over:
>>> EXPECT_TRUE(kExpected == foo) << "Some message about " <<
>>> kExpected.InSecondsF() << " and " << foo.InSecondsF();
>>> error: Value of: kExpected == foo
>>>   Actual: false
>>> Expected: true
>>> Some message about 21.0 and 22.0
>>>
>>> Guaranteed I won't write that message every time and then I end up with a
>>> simple true/false dump instead of the erroneous values.
>>>
>>> On Thu, Aug 20, 2009 at 6:49 PM, Matt Perry wrote:
>>>
 Andrew wants to be able to do:
   DCHECK_EQ(expected_time_delta, time_delta);
 This can't be done without operator<< support.


 On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:

> +1 for Peter's suggestion.
> TimeDelta has an internal accuracy of microseconds.  What
> resolution/scaling do you want to print in a check?  Sometimes it is
> minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
> microseconds :-/.
>
> Explicit conversion as suggested doesn't seem that painful IMO.
>
> Jim
>
>
> On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting 
> wrote:
>
>> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus <
>> scher...@chromium.org> wrote:
>>
>>> Any opposition to globally declaring an operator<< ostream overload
>>> for TimeDelta in base/time.h?
>>>
>>
>> This will pull the stream headers into all files that use time.h.  Is
>> that going to bloat any code or cost compile time?
>>
>> Is there another easy solution like doing DCHECK() << "TimeDelta was:
>> " << myTimeDelta.asInt64OrWhatever()?
>>
>> PK
>>
>>
>>
>
> >
>

>>>
>>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Matt Perry]

Defining operator<< is fine. Other types do this:
http://www.google.com/codesearch?as_q=operator<<&btnG=Search+Code&hl=en&as_lang=&as_license_restrict=i&as_license=&as_package=src.chromium.org/svn/trunk/src&as_filename=\.h&as_case=

string16 is a good example.

I say just use iosfwd and add the operator.

On Thu, Aug 20, 2009 at 10:10 PM, Jim Roskind  wrote:

> Looking at the example you gavehow about:
> EXPECT_EQ(kExpected.InMilliseconds(), foo.InMilliseconds());
>
> Is that really that painful to write?
>
> ...and you could get all the microseconds to compare if you wanted to via
> ...InMicroseconds().
>
> I suspect you don't really want absolute comparisons at the microsecond
> level, and more likely you'd want something like;
>
> EXPECT_LT((kEpected - foo).InMilliseconds(), 20).
>
> ...but if you really wanted the example you cited, the first line seems
> relatively short.
>
> Jim
>
> On Thu, Aug 20, 2009 at 7:13 PM, Andrew Scherkus wrote:
>
>> I know microseconds aren't a very user-friendly format, but for unit tests
>> and DCHECKs I'm more interested in whether the assertion is simply true.
>>
>> Perhaps I'm lazy but I'd prefer:
>> EXPECT_EQ(kExpected, foo);
>> error: Value of: foo
>>   Actual: 2100
>> Expected: kExpected
>> Which is: 2200
>>
>> ...over:
>> EXPECT_TRUE(kExpected == foo) << "Some message about " <<
>> kExpected.InSecondsF() << " and " << foo.InSecondsF();
>> error: Value of: kExpected == foo
>>   Actual: false
>> Expected: true
>> Some message about 21.0 and 22.0
>>
>> Guaranteed I won't write that message every time and then I end up with a
>> simple true/false dump instead of the erroneous values.
>>
>> On Thu, Aug 20, 2009 at 6:49 PM, Matt Perry wrote:
>>
>>> Andrew wants to be able to do:
>>>   DCHECK_EQ(expected_time_delta, time_delta);
>>> This can't be done without operator<< support.
>>>
>>>
>>> On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:
>>>
 +1 for Peter's suggestion.
 TimeDelta has an internal accuracy of microseconds.  What
 resolution/scaling do you want to print in a check?  Sometimes it is
 minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
 microseconds :-/.

 Explicit conversion as suggested doesn't seem that painful IMO.

 Jim


 On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting 
 wrote:

> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus <
> scher...@chromium.org> wrote:
>
>> Any opposition to globally declaring an operator<< ostream overload
>> for TimeDelta in base/time.h?
>>
>
> This will pull the stream headers into all files that use time.h.  Is
> that going to bloat any code or cost compile time?
>
> Is there another easy solution like doing DCHECK() << "TimeDelta was: "
> << myTimeDelta.asInt64OrWhatever()?
>
> PK
>
>
>

 

>>>
>>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Jim Roskind]

Looking at the example you gavehow about:
EXPECT_EQ(kExpected.InMilliseconds(), foo.InMilliseconds());

Is that really that painful to write?

...and you could get all the microseconds to compare if you wanted to via
...InMicroseconds().

I suspect you don't really want absolute comparisons at the microsecond
level, and more likely you'd want something like;

EXPECT_LT((kEpected - foo).InMilliseconds(), 20).

...but if you really wanted the example you cited, the first line seems
relatively short.

Jim

On Thu, Aug 20, 2009 at 7:13 PM, Andrew Scherkus wrote:

> I know microseconds aren't a very user-friendly format, but for unit tests
> and DCHECKs I'm more interested in whether the assertion is simply true.
>
> Perhaps I'm lazy but I'd prefer:
> EXPECT_EQ(kExpected, foo);
> error: Value of: foo
>   Actual: 2100
> Expected: kExpected
> Which is: 2200
>
> ...over:
> EXPECT_TRUE(kExpected == foo) << "Some message about " <<
> kExpected.InSecondsF() << " and " << foo.InSecondsF();
> error: Value of: kExpected == foo
>   Actual: false
> Expected: true
> Some message about 21.0 and 22.0
>
> Guaranteed I won't write that message every time and then I end up with a
> simple true/false dump instead of the erroneous values.
>
> On Thu, Aug 20, 2009 at 6:49 PM, Matt Perry wrote:
>
>> Andrew wants to be able to do:
>>   DCHECK_EQ(expected_time_delta, time_delta);
>> This can't be done without operator<< support.
>>
>>
>> On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:
>>
>>> +1 for Peter's suggestion.
>>> TimeDelta has an internal accuracy of microseconds.  What
>>> resolution/scaling do you want to print in a check?  Sometimes it is
>>> minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
>>> microseconds :-/.
>>>
>>> Explicit conversion as suggested doesn't seem that painful IMO.
>>>
>>> Jim
>>>
>>>
>>> On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting wrote:
>>>
 On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus >>> > wrote:

> Any opposition to globally declaring an operator<< ostream overload for
> TimeDelta in base/time.h?
>

 This will pull the stream headers into all files that use time.h.  Is
 that going to bloat any code or cost compile time?

 Is there another easy solution like doing DCHECK() << "TimeDelta was: "
 << myTimeDelta.asInt64OrWhatever()?

 PK



>>>
>>> >>>
>>>
>>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Erik Kay]

On Thu, Aug 20, 2009 at 7:13 PM, Andrew Scherkus wrote:
> I know microseconds aren't a very user-friendly format, but for unit tests
> and DCHECKs I'm more interested in whether the assertion is simply true.
> Perhaps I'm lazy but I'd prefer:
> EXPECT_EQ(kExpected, foo);
> error: Value of: foo
>   Actual: 2100
> Expected: kExpected
> Which is: 2200
> ...over:
> EXPECT_TRUE(kExpected == foo) << "Some message about " <<
> kExpected.InSecondsF() << " and " << foo.InSecondsF();
> error: Value of: kExpected == foo
>   Actual: false
> Expected: true
> Some message about 21.0 and 22.0
> Guaranteed I won't write that message every time and then I end up with a
> simple true/false dump instead of the erroneous values.

You could also solve this by writing EXPECT_EQ_TIME().  It's not as
elegant, but I worry that your argument could be applied to almost any
type in our system.

Erik


> On Thu, Aug 20, 2009 at 6:49 PM, Matt Perry  wrote:
>>
>> Andrew wants to be able to do:
>>   DCHECK_EQ(expected_time_delta, time_delta);
>> This can't be done without operator<< support.
>>
>> On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:
>>>
>>> +1 for Peter's suggestion.
>>> TimeDelta has an internal accuracy of microseconds.  What
>>> resolution/scaling do you want to print in a check?  Sometimes it is
>>> minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
>>> microseconds :-/.
>>> Explicit conversion as suggested doesn't seem that painful IMO.
>>> Jim
>>>
>>> On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting 
>>> wrote:

 On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus 
 wrote:
>
> Any opposition to globally declaring an operator<< ostream overload for
> TimeDelta in base/time.h?

 This will pull the stream headers into all files that use time.h.  Is
 that going to bloat any code or cost compile time?
 Is there another easy solution like doing DCHECK() << "TimeDelta was: "
 << myTimeDelta.asInt64OrWhatever()?
 PK

>>>
>>>
>>>
>>
>
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Andrew Scherkus]

I know microseconds aren't a very user-friendly format, but for unit tests
and DCHECKs I'm more interested in whether the assertion is simply true.

Perhaps I'm lazy but I'd prefer:
EXPECT_EQ(kExpected, foo);
error: Value of: foo
  Actual: 2100
Expected: kExpected
Which is: 2200

...over:
EXPECT_TRUE(kExpected == foo) << "Some message about " <<
kExpected.InSecondsF() << " and " << foo.InSecondsF();
error: Value of: kExpected == foo
  Actual: false
Expected: true
Some message about 21.0 and 22.0

Guaranteed I won't write that message every time and then I end up with a
simple true/false dump instead of the erroneous values.

On Thu, Aug 20, 2009 at 6:49 PM, Matt Perry  wrote:

> Andrew wants to be able to do:
>   DCHECK_EQ(expected_time_delta, time_delta);
> This can't be done without operator<< support.
>
>
> On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:
>
>> +1 for Peter's suggestion.
>> TimeDelta has an internal accuracy of microseconds.  What
>> resolution/scaling do you want to print in a check?  Sometimes it is
>> minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
>> microseconds :-/.
>>
>> Explicit conversion as suggested doesn't seem that painful IMO.
>>
>> Jim
>>
>>
>> On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting wrote:
>>
>>> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus 
>>> wrote:
>>>
 Any opposition to globally declaring an operator<< ostream overload for
 TimeDelta in base/time.h?

>>>
>>> This will pull the stream headers into all files that use time.h.  Is
>>> that going to bloat any code or cost compile time?
>>>
>>> Is there another easy solution like doing DCHECK() << "TimeDelta was: "
>>> << myTimeDelta.asInt64OrWhatever()?
>>>
>>> PK
>>>
>>>
>>>
>>
>> >>
>>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Matt Perry]

Andrew wants to be able to do:
  DCHECK_EQ(expected_time_delta, time_delta);
This can't be done without operator<< support.

On Thu, Aug 20, 2009 at 6:46 PM, Jim Roskind  wrote:

> +1 for Peter's suggestion.
> TimeDelta has an internal accuracy of microseconds.  What
> resolution/scaling do you want to print in a check?  Sometimes it is
> minutes, sometimes seconds, sometimes milliseconds, I doubt that we want
> microseconds :-/.
>
> Explicit conversion as suggested doesn't seem that painful IMO.
>
> Jim
>
>
> On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting wrote:
>
>> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus 
>> wrote:
>>
>>> Any opposition to globally declaring an operator<< ostream overload for
>>> TimeDelta in base/time.h?
>>>
>>
>> This will pull the stream headers into all files that use time.h.  Is that
>> going to bloat any code or cost compile time?
>>
>> Is there another easy solution like doing DCHECK() << "TimeDelta was: " <<
>> myTimeDelta.asInt64OrWhatever()?
>>
>> PK
>>
>>
>>
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Jim Roskind]

+1 for Peter's suggestion.
TimeDelta has an internal accuracy of microseconds.  What resolution/scaling
do you want to print in a check?  Sometimes it is minutes, sometimes
seconds, sometimes milliseconds, I doubt that we want microseconds :-/.

Explicit conversion as suggested doesn't seem that painful IMO.

Jim

On Thu, Aug 20, 2009 at 4:02 PM, Peter Kasting wrote:

> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus wrote:
>
>> Any opposition to globally declaring an operator<< ostream overload for
>> TimeDelta in base/time.h?
>>
>
> This will pull the stream headers into all files that use time.h.  Is that
> going to bloat any code or cost compile time?
>
> Is there another easy solution like doing DCHECK() << "TimeDelta was: " <<
> myTimeDelta.asInt64OrWhatever()?
>
> PK
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Paweł Hajdan Jr .]

On Thu, Aug 20, 2009 at 16:02, Peter Kasting  wrote:

> On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus wrote:
>
>> Any opposition to globally declaring an operator<< ostream overload for
>> TimeDelta in base/time.h?
>>
>
> This will pull the stream headers into all files that use time.h.  Is that
> going to bloat any code or cost compile time?
>

You can use  and implement the operator in the .cc file. I actually
recommend doing it that way.

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Peter Kasting]

On Thu, Aug 20, 2009 at 3:33 PM, Andrew Scherkus wrote:

> Any opposition to globally declaring an operator<< ostream overload for
> TimeDelta in base/time.h?
>

This will pull the stream headers into all files that use time.h.  Is that
going to bloat any code or cost compile time?

Is there another easy solution like doing DCHECK() << "TimeDelta was: " <<
myTimeDelta.asInt64OrWhatever()?

PK

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Overloading operator<< for TimeDelta

[Mark Mentovai]

Andrew Scherkus wrote:
> Any opposition to globally declaring an operator<< ostream overload for
> TimeDelta in base/time.h?
> According to style guide it needs to be fully justified, but it'd be nice to
> use DCHECK_xx/EXEPCT_xx/ASSERT_xx with TimeDeltas.

I think this is fine.

Mark

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---