Allow locally set time references for Date and Faker[T]

[bchavez]

Allows locally set time references for Date calculations instead of global statics. See Faker[T].UseDateTimeReference() and Date.LocalSystemClock. Alternative PR to #492.

[garcipat]

I like the setting as with the seed. This makes logically a lot of sense and is cleaner then having a property to set.

[bchavez]

I like the setting as with the seed. This makes logically a lot of sense and is cleaner then having a property to set.

@garcipat I think I agree. Perhaps we can [Obsolete] .UseSeed(int) in favor of .UseSeed(int, refDate); but I would probably still keep Faker<T>.UseDateTimeReference() since it could be used with Faker<T>.Clone() more flexibly.

[garcipat]

I like the setting as with the seed. This makes logically a lot of sense and is cleaner then having a property to set.

@garcipat I think I agree. Perhaps we can [Obsolete] .UseSeed(int) in favor of .UseSeed(int, refDate); but I would probably still keep Faker<T>.UseDateTimeReference() since it could be used with Faker<T>.Clone() more flexibly.

@bchavez No what I meant I like the UseDateTimeReference method over setting it as a property as I did it in the PR #492. I think you should keep the UseSeed separated. Both make something completly different and it's more a fluent synthax like that.

[garcipat]

TODO: Also write some tests for NET6 only for DateOnly and TimeOnly since we might have missed a code change in #492

@bchavez If you like I write a test for every method. I dont know if you need to have all of them covered with the relative or if one with DateOnly and one with TimeOnly already covers enough. I can do it in the other PR and you can take it over and tweak it. To spare you some time.

[bchavez]

API surface so far:

//configuring per Faker[T] instance
var fakerT = new Faker<T>()
     .UseSeed(1337)
     .UseDateTimeReference(new DateTime(2022, 1, 23))

//configuring per Faker instance
var faker = new Faker()
{
     Random = new Randomizer(1337),
     DateTimeReference = new DateTime(2022, 1, 23)
}

//configuring per Date dataset instance
var dateDataSet = new Date()
{
     Random = new Randomizer(1337),
     LocalSystemClock = () => new DateTime(2022, 1, 23)
}

I'm thinking so far, seems DateTimeReference is a bit generic; maybe something like:

• FixedTimeReference or
• UseLocalSystemClock or
• DateTimeAnchor

is more semantically accurate. The only good thing about DateTimeReference is that it's kind of discoverable when f => f.Date is typed in VIsual Studio.

[garcipat]

@bchavez Now I'm really confused 😅. I cannot work on this PR I think (or can I? thought I cannot push here, or can I just don't make own branches?). Therefore I took over all your changes to the #492 and added tests and things there since thats the only thing I can do.

[garcipat]

My recent changes on my branch are not in the PR #492 anymore since you closed it, but on the same branch. Just becasue I think now you did not pull over the recent changes where I took over most of your changes in this branch and its mixed up now :(

[bchavez]

@garcipat ; what particular tests do you want me to pull over from #492 ?

[garcipat]

@garcipat ; what particular tests do you want me to pull over from #492 ?

@bchavez the PR does not contain my recent changes since it was closed before that.
I just want to prevent that you take over code that you wrote better and is replaced even on my branch.
Should I reopen the PR? or is that even possible?

I'm talking about the changes I did yesterday as promised. For all the operations that reference the GetTimeReference() you wrote I added a test for regular and Offset methods to cover all the cases (mainly here) and also the corrections in the comments that reference DateTime.Now.

[garcipat]

Ah I see you wrote the tests yourself now. Well I dont know anymore then. I think I will leave it to you since it seems I'm only doing everything in parallel while trying to help adding the missing things.

[bchavez]

@garcipat hmm. closing the PR should not have any impact on what was committed; or what previously & currently exists on your fork:

• https://github.com/garcipat/Bogus/tree/reference-date

The only way something could be "lost" is if you pushed some code locally to your fork that wasn't totally in-sync with what you pushed to your fork/remote.

That said tho, I think we should continue to focus energy on PR #500 since this is the PR we will go with; I'm just putting the final touches on the public API surface:

• Faker<T>.UseDateTimeReference()
• Faker<T>.UseFixedTimeReference()
• Faker<T>.UseDateTimeAnchor()
• Faker<T>.UseLocalSystemClock()

Right now, kinda leaning to renaming .UseDateTimeReference() -> .UseDateTimeAnchor()

[garcipat]

@garcipat hmm. closing the PR should not have any impact on what was committed; or what previously & currently exists on your fork:

• https://github.com/garcipat/Bogus/tree/reference-date

The only way something could be "lost" is if you pushed some code locally to your fork that wasn't totally in-sync with what you pushed to your fork/remote.

That said tho, I think we should continue to focus energy on PR #500 since this is the PR we will go with; I'm just putting the final touches on the public API surface:

• Faker<T>.UseDateTimeReference()
• Faker<T>.UseFixedTimeReference()
• Faker<T>.UseDateTimeAnchor()
• Faker<T>.UseLocalSystemClock()

Right now, kinda leaning to renaming .UseDateTimeReference() -> .UseDateTimeAnchor()

Can I push to that branch? With "focus energy in that branch" would require me to be able to do that. Otherwise I cannot help. I can push the same changes there to if its technically possible. I will try I guess.

Regarding the proposals, I would lean to the firstor the third. The seconds feels very technical and not so semantically and The "Local" in the last could be confusing about the Local/Utc DateTimeKind because of the wording.

One thing that would give UseDateTimeReference a +1 is, that all arguments in the Api are called refDate. Therefore this would be aligned with that.

Edit: Yes just tried. I can't commit to this branch either. You have to take the changes over manually from my branch if you want them.

[304NotModified]

@bchavez what should be done to get this feature merged?

It still would be great if we had this!

[bchavez]

@304NotModified i think the first thing is to get this rebased to master. then finally, one final review push/reading through the context on this ticket to make sure I'm not missing anything. and also some README documentation changes.

Currently, getting taxes done is my main blocker on any open-source work/releases atm. I'll try to get this branch rebased this weekend.

[bchavez]

@304NotModified and @garcipat, i think this PR is just about ready. I'm planning on merging it next weekend.

@garcipat, I pulled in your tests from PR #508 and gave you credit on the HISTORY.md file.

Let me know if there's any major concerns with the PR before we merge next weekend. README.md documentation on Deterministic Dates and Times should be current with this PR.

[304NotModified]

Excellent news!

[garcipat]

@304NotModified and @garcipat, i think this PR is just about ready. I'm planning on merging it next weekend.

@garcipat, I pulled in your tests from PR #508 and gave you credit on the HISTORY.md file.

Let me know if there's any major concerns with the PR before we merge next weekend. README.md documentation on Deterministic Dates and Times should be current with this PR.

Thank you so much. I feel honored and am very happy that I could contribute something!

[bchavez]

Changes are released in Bogus v35.5.0:

• https://www.nuget.org/packages/Bogus/35.5.0

Let me know if there are any issues. 👍