search

Balzanka / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Add explicitly named static factories to Stopwatch instead of the ambiguous ctor #1307

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
How about we deprecate the constructor and then add two static creation methods 
named something like "newUnstartedInstance()" and "newStartedInstance()"?

This cleanup would be a little hairy, and the names aren't quite perfect, but 
maybe it's worth doing? At least Stopwatch is still @Beta.

(Note we can't just throw an ISE when calling methods on a non-started 
stopwatch since there are several use-cases where you'd expect a non-started 
stopwatch to return 0 for elapsedMillis())

Let's discuss further at API review.

Original issue reported on code.google.com by kak@google.com on 26 Feb 2013 at 9:23

GoogleCodeExporter commented 9 years ago 
There are two constructors in the master branch: one that takes a ticker and 
the other that specifies the system ticker. Both of these create an unstarted 
Stopwatch. Are you proposing to remove the ability to specify our own ticker?

Original comment by dharkn...@gmail.com on 18 Mar 2013 at 12:50

GoogleCodeExporter commented 9 years ago 
@dharkness: Nope...we'd remove the 2 constructors and add 4 static factory 
methods:

public static Stopwatch newStartedInstance() { ... }
public static Stopwatch newStartedInstance(Ticker ticker) { ... }
public static Stopwatch newUnstartedInstance() { ... }
public static Stopwatch newUnstartedInstance(Ticker ticker) { ... }

Original comment by kurt.kluever on 18 Mar 2013 at 4:16

GoogleCodeExporter commented 9 years ago 
We're looking for feedback from users on whether this change is seen as worth 
the hassle of fixing all your existing "new Stopwatch()" calls.

Original comment by kevinb@google.com on 18 Mar 2013 at 2:15

GoogleCodeExporter commented 9 years ago 
I for one think that it's worth fixing. In my codebase it'll be largely a 
find-replace for the started ones. Definitely a clarity win.

Original comment by joe.j.kearney on 19 Mar 2013 at 8:43

GoogleCodeExporter commented 9 years ago 
Agreed with #4.

Original comment by stephan...@gmail.com on 19 Mar 2013 at 9:03

GoogleCodeExporter commented 9 years ago 
Great, thanks for the feedback Stephen + Joe.  I've just put up a G+ post about 
this topic, so we'll see what everyone thinks:
https://plus.google.com/118010414872916542489/posts/8XeVCd7iKxo

Original comment by kak@google.com on 19 Mar 2013 at 8:05

GoogleCodeExporter commented 9 years ago 
I think I'll push for Stopwatch.createStarted() and createUnstarted().

Fun fact from that G+ thread: 78% of Stopwatches are started immediately after 
construction!

Original comment by kak@google.com on 20 Mar 2013 at 9:28

GoogleCodeExporter commented 9 years ago 
OK, we've decided to add Stopwatch.createStarted() and createUnstarted() and 
then ultimately deprecate and remove the Stopwatch constructors! Thanks 
everyone for the valuable discussions :-)

Original comment by kak@google.com on 26 Mar 2013 at 6:35

GoogleCodeExporter commented 9 years ago 
I'm late to the party but +1 nonetheless!

Original comment by dharkn...@gmail.com on 20 Jan 2014 at 11:11

GoogleCodeExporter commented 9 years ago 
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:13

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08