Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recommendation: Mark Id constructor as public #63

Open
jhimes144 opened this issue Dec 27, 2024 · 6 comments
Open

Recommendation: Mark Id constructor as public #63

jhimes144 opened this issue Dec 27, 2024 · 6 comments
Assignees

Comments

@jhimes144
Copy link

One of the primary benefits of snowflakes is the ability to query the database for ids that were created after a certain timeframe, without relying on a separate DateTime column. This is how discord does their queries, as 64bit comparisons are faster than date time comparisons. I saw this issue that was closed #60 and I partially disagree with the answer.

We need to be able to create an ID of which represents a point of time, to generate a query to get all items past that moment.

@RobThree
Copy link
Owner

I think a CreateId(DateTimeOffset dateTime) overload would be a better option as it will at least be able to guarantee the generator-id is a 'known/existing' one. But I'll have to sit on this for a bit to think of any complications that may bring.

@jhimes144
Copy link
Author

That sounds good! Great library by the way, thank you so much for making it. Its written well. We ended up forking it for our needs.

@RobThree RobThree self-assigned this Dec 27, 2024
@RobThree
Copy link
Owner

RobThree commented Dec 27, 2024

Out of curiosity: what changes did you make? Or was it "just" making the constructor public?

@jhimes144
Copy link
Author

I created a method in the ID generator to create a id from a DateTimeOffset, I did not end up making any changes to the ID, to keep its immutable nature.

@RobThree
Copy link
Owner

RobThree commented Dec 28, 2024

The problem I'm facing is: I can't use CreateIdImpl without major refactoring because all clock-handling (clock going backwards etc.) is going out the window with this method. And I don't even know what to do with the sequence; I could take it as a second argument but that would make the user responsible for generating the sequence part. All that's left for the IdGenerator to do then, is set the generator-bits and out the datetime/sequence bits into place.

Also, passing a DateTimeOffset wouldn't work, we'd need an ITimeSource to be passed because we don't know what the tick duration is (can be anything). And that, in turn, would make invoking CreateId a lot more complicated because you'd need to first create an ITimeSource, set it to some date in the past, determine the sequence you desire and then call CreateId with the ITimeSource and generated sequence as arguments.

It's not impossible but overly complicated I think. However. a 'workaround' could be creating another IdGenerator instance with an ITimeSource you can set to your desired date. Normally I don't recommend having more than one IdGenerator in a process/task/thread/whatever and recommend a singleton. But I could see a usecase in this particular case. You could set the date of your custom ITimeSource, generate a bunch of ID's and dispose the IdGenerator. Only issue with this is that you must create Id's chronologically and can't generate them for random times because that will trigger the "Clock moved backwards" mechanism. Or you'd have to instantiate a new IdGenerator for each )non-chronological) Id...

Thoughts are welcome.

@jhimes144
Copy link
Author

I hear you, with the way everything is structured it sounds like it's going to be hard to implement in a succinct way. It worked for me because I got rid of all supporting classes and hard coded everything for my use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants