Consider adding IdGenerator#CreateFromDateTime about idgen

[glen-84]

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 InvalidSystemClockExceptions if I keep the imports correctly ordered (unless I'm forgetting something).

I'll see how it goes. 👍

[RobThree]

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!

[glen-84]

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);

Implementation

public class TimeSource : ITimeSource
{
    public DateTimeOffset Epoch { get; }
    public TimeSpan TickDuration { get; }

    private DateTimeOffset? dateTime;

    private DateTimeOffset DateTime { get => this.dateTime ?? DateTimeOffset.UtcNow; }

    public TimeSource(DateTimeOffset epoch, TimeSpan tickDuration)
    {
        this.Epoch = epoch;
        this.TickDuration = tickDuration;
    }

    public void SetDateTime(DateTimeOffset dateTime)
    {
        this.dateTime = dateTime;
    }

    public long GetTicks() => (this.DateTime - this.Epoch).Ticks / this.TickDuration.Ticks;
}

Seems to work.

You can close this issue if you don't think that something similar should be built into the library.

Thanks! 🙂

[RobThree]

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);

Implementation

public class MyTimeSource : ITimeSource
{
    public DateTimeOffset Epoch { get; } = new DateTime(2002, 1, 1, 0, 0, 0, DateTimeKind.Utc);
    public TimeSpan TickDuration { get; } = TimeSpan.FromMilliseconds(1);

    private DateTimeOffset _dateTime;

    public void SetDateTime(DateTimeOffset dateTime) => _dateTime = dateTime;

    public long GetTicks() => (_dateTime - Epoch).Ticks / TickDuration.Ticks;
}

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.

[RobThree]

Great 👍