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

spec question #12

Open
benoitc opened this issue Oct 6, 2017 · 4 comments
Open

spec question #12

benoitc opened this issue Oct 6, 2017 · 4 comments

Comments

@benoitc
Copy link

benoitc commented Oct 6, 2017

The spec for the timestamp is integer() but it seems that you can pass {integer(), integer()} like you do in the run function. Is this expected ?

@jlouis
Copy link
Owner

jlouis commented Oct 6, 2017

It can be any term as long as the terms are ordered (the order is the "time" at which something happened). Uniqueness is needed in order to make sure that in the case of timestamps colliding, we can still order them.

Historically, this was an erlang:now() triple which is guaranteed to have the two above criteria. However, since OTP 17.0, we needed to add a unique_integer() call in there to obtain uniqueness over the terms for backwards compatibility.

The QuickCheck model works with its own integer() representation of time here and doesn't really care about anything but the above ordering constraints.

Looking ahead, I think it would be fine to document the above criteria as for a behavior you want on the values. The simplest way is just to call sv:timestamp() in your own code, but there may be users who want to use their own ordering for some reason or the other. In any case, there are some room for improvement here.

@benoitc
Copy link
Author

benoitc commented Oct 6, 2017

I'm mostly concerned by the spec as i'm not sur how dialyzer will react with some strict settings. I will test and report

@jlouis
Copy link
Owner

jlouis commented Oct 6, 2017

Feel free to provide a patch which fixes the spec if needed. The spec isn't correct if it is integer(). Perhaps we should just add a timestamp() type to the sv module and let it contain the valid alternative types in there. e.g., something along the lines of

-type timestamp() :: integer() | {integer(), integer()} | ...

Or whatever types I assume in a modern Erlang implementation.

@benoitc
Copy link
Author

benoitc commented Oct 6, 2017

i will send you a PR in the morning

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