Make usable zero value of clockwork.Clock

[rekby]

for this - change clockwork.Clock type from interface to struct or pointer with call system time functions by default (for zero value).

It allow make my own types usable with zero value without wrap time calls in my code.

[DPJacques]

NewFakeClockAt(t time.Time) Is provided if you need a specific starting time.

NewFakeClock() returns a time that is not the zero the zero time or the unix epoch by design, due to their likelihood to be overlooked by default values. You should not rely on it's default value.

[rekby]

I suggest to use zero value of clockwork.Clock as clockwork.NewRealClock() - for skip clock initialization in production code.

for example wrapper with same behavior: https://gist.github.com/rekby/a7f3e679e47604b28d22c7736d0ff388

[sagikazarmark]

@rekby this is certainly an interesting proposal, but even your wrapper relies on an interface for allowing alternate implementations, so we can't just throw it away.

We can, however, add such a wrapper to the library (called DefaultClock or something). One could use that instead of the interface so that there is no need for initialization.

Let me play with that wrapper of yours a little.

[DPJacques]

Gotcha, your issue title "usable zero value" confused me. I think I now understand that you are asking for custom clocks.

Changing this interface to a struct would break most users, because the interface is passed as a <pointer, type> whereas the struct would be passed by value copy unless users changed it to a pointer to struct throughout their code base. It is unlikely that change will be made.

If you want to accomplish what you are seeking, NewFakeClockAt(time.Time{}) does exactly what you are asking for with the current code. I don't really think providing alternatives types and wrappers should be added just for this feature request.

[sagikazarmark]

@DPJacques I believe this is what @rekby is asking for:

type MyStruct struct{
    Clock clockwork.Clock
}

var mystruct MyStruct

// Use Clock without initialization
mystruct.Clock.Now()

It should be possible with an alternate name, but we may have to change the real clock implementation to use values instead of pointers (which doesn't sound all that bad TBH).

[DPJacques]

Gotcha, the code helps understand a bit.

I maintain my position that I don't think avoiding initialization is a compelling requirement, in light of:

1. Adding types or wrappers with alternative names feels like more overhead than it is worth to support avoiding initialization (see code block below). Alternative ways to do things are usually tech debt flags.
2. Myself and my colleagues have questioned the value of optimizing for skipping initialization as a feature, as it is error prone in complex code. The best public example I can immediately point to is proto3's default value is zero convention as opposed to porto2's default value is nil convention. If I understand correctly proto3 authors (and Go proto authors) are now trying to undo/unwind that decision, making proto3 more proto2-like and adding an API around the original proto struct implementation.
3. The currently thin wrapper of realClock minimized performance overhead in production. I'd be remiss to add logic to the various clock functions as that seems costly at scale. Update: maybe this wouldn't be required if adding new classes and wrappers, but that speaks to point 1.

Requiring the code block above to be the following does not seem egregious to me:

type MyStruct struct{
    Clock clockwork.Clock
}

mystruct := MyStruct{clock: clockwork.NewRealClock()}

mystruct.Clock.Now()

And I admit I'd be ok requiring users with strong opinions to roll their own wrapper if it means the API stays clean.

Not necessarily my call, just my $0.02 :)

[sagikazarmark]

See the linked PR.

I myself is not convinced yet that this belongs to this package, but I wanted to play with the idea.

[DPJacques]

Totally makes sense. I'm glad I am not the only one who needs to code a bit to convince themselves if something is a good idea or not :smile:

I did my best to provide reasoning for my stance above. I don't think I could do any better than the PR you put out, although that reinforces my position: it feels like a lot of code for not a lot of benefit :/

[DPJacques]

Closing this out since clockwork.Clocks status as an interface is the thing that makes it usage by realClock or fakeClock. This is a common use of interfaces in Go, and approaches to make a usable default value are hard to migrate to and use. The benefit gained is minimal in comparison.