Closed glen-84 closed 1 year ago
I'm afraid it's a little more complicated than that. We don't use a DateTime
but 'ticks' that can be defined to be anything; be it a millisecond (default), a second or even a day or nanosecond (hardware support etc. permitting). The ITimeSource
returns the number of these 'ticks' since an epoch. With your proposed IdGenerator#CreateFromDateTime(DateTime dateTime)
method it implies the timesource given during initialization (constructor) would be ignored and 'overridden' with the given dateTime
. However, there's no way to 'convert' that value to 'ticks' since a 'tick' can be anything.
I do see a few options on how to 'fix' this. The simplest would be to change the proposed method to IdGenerator#CreateFromDateTime(long ticks)
. That way, we leave it to the caller to figure out how the date/time should be converted to ticks. However, it would still mean a (dummy) timesource would still need to be provided to the constructor (or a default timesource will still be created but unused).
Another option would be to pass an ITimeSource
to the proposed method: IdGenerator#CreateFromDateTime(ITimeSource timeSource)
but then you could just as well pass the timesource to the constructor and 'set' it's date/time to whatever value you want before calling CreateId()
. But that would introduce thread-safety issues.
I see a few other maybe-possibilities but most would require quite a big refactor and breaking changes to the current API IdGen currently provides.
If you have any suggestions they're more than welcome!
I was initially going to suggest IdGenerator#CreateFromTicks
, and then I changed it to CreateFromDateTime
, but I see what the problem is.
or a default timesource will still be created but unused
Not ideal, but I wonder if it's a show stopper?
but then you could just as well pass the timesource to the constructor
The problem with this is that you have to create a new IdGenerator for every ID that you generate, which seems inefficient.
I thought of an alternative: Allow the date-time to be set on the time source directly:
var epoch = new DateTime(2002, 1, 1, 0, 0, 0, DateTimeKind.Utc);
var nextDate = new DateTime(2024, 2, 2, 2, 2, 2, DateTimeKind.Utc);
var idGeneratorOptions = new IdGeneratorOptions(
null,
new TimeSource(epoch, TimeSpan.FromMilliseconds(1)),
SequenceOverflowStrategy.SpinWait);
var generator = new IdGenerator(0, idGeneratorOptions);
var id1 = generator.CreateId();
var decoded1 = generator.FromId(id1);
// Change date-time.
((TimeSource)generator.Options.TimeSource).SetDateTime(nextDate);
var id2 = generator.CreateId();
var decoded2 = generator.FromId(id2);
Seems to work.
You can close this issue if you don't think that something similar should be built into the library.
Thanks! 🙂
The problem with this is that you have to create a new IdGenerator for every ID that you generate, which seems inefficient.
No you don't as you figured out yourself. As long as you have an ITimeSource
that lets you set the time for (like MyTimeSource.SetTime(<somedate>)
) you should be good to go. (Basically, the MockTimeSource
is what you want).
I think your example code is a little more complicated than necessary:
var mytimesource = new MyTimeSource();
var idGeneratorOptions = new IdGeneratorOptions(
null,
mytimesource,
SequenceOverflowStrategy.SpinWait
);
var generator = new IdGenerator(0, idGeneratorOptions);
// For each ID to generate "set the time" and create the ID:
mytimesource.SetDateTime(/* <somevalue> */);
var id = generator.CreateId();
var decoded = generator.FromId(id1);
Above code is untested.
The problem with this approach is that there's a concurrency issue; when two threads set the time of the mytimesource
you're likely to get incorrect results.
Basically, the MockTimeSource is what you want
The MockTimeSource
doesn't support an epoch/tickDuration, and is also in a separate assembly related to tests.
I think your example code is a little more complicated than necessary
Sure, I can hard-code defaults if necessary.
The problem with this approach is that there's a concurrency issue; when two threads set the time of the mytimesource you're likely to get incorrect results.
This is for a console app performing data imports, so it shouldn't be an issue unless/until I decide to run something concurrently or in parallel. At the moment I'm passing each importer its own IdGenerator instance, so they shouldn't interfere with each other, and I shouldn't see InvalidSystemClockException
s if I keep the imports correctly ordered (unless I'm forgetting something).
I'll see how it goes. 👍
Great 👍
When importing data, it's useful to create IDs for a specific date/time, but as far as I know, this requires constructing a custom time source, options, and generator for every generated ID (example).
A method like
IdGenerator#CreateFromDateTime(DateTime dateTime)
would make this a lot more efficient.