jonboulle / clockwork

a fake clock for golang
Apache License 2.0
656 stars 57 forks source link

Added a singleton instance of the real clock. #14

Closed kulti closed 4 years ago

kulti commented 6 years ago

When we use NewRealClock() from a lot of place, it will be helpful to have one instance for all usage.

sagikazarmark commented 4 years ago

@kulti honestly, it feels like a bad practice to me. Relying on a global and exported variable, that other code could simply just set to nil is risky.

I'm also not convinced that there is a real performance gain here.

I'd suggest creating a global variable in your software and use it from there.

As an alternative, I could accept a PR that doesn't use exported global variables, for example:

var realClockSingleton = NewRealClock()

func RealClock() Clock {
    return realClockSingleton
}

I'm still not convinced it's necessary to have, but that's a PR I could accept. Thanks!

kulti commented 4 years ago

@sagikazarmark thank you for your reply!

I've made this PR 1,5 years ago. My mind has changed a lot since. So, I can't remember why I thought this PR is useful. It's simpler to create an internal package with a global realclock if needed.

Using global variables is generally a bad idea for many reasons. But the clock, logger, metrics are essentially global. So, that is not a big deal to have a clock in the global scope.

At now I'm using another package — https://github.com/cabify/timex. I think it implements mock from time in a good way.