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

SimpleTimeline interface polishing #34

Closed
elidupree opened this issue Aug 30, 2017 · 1 comment
Closed

SimpleTimeline interface polishing #34

elidupree opened this issue Aug 30, 2017 · 1 comment

Comments

@elidupree
Copy link
Owner

elidupree commented Aug 30, 2017

  • Should it really be able to report the time/event that set the data to its current value? This makes serialized snapshots bigger, and is often unnecessary, or misleading (as I found in simple_diffusion when I modified a SimpleTimeline just to change a field that was different from the one that was based on the last change time). If it DOES report, it should presumably report the EventHandle instead of just the ExtendedTime. Originally, I only included this feature because it happened to be easy to provide, but it adds some annoyances, and storing the time as a 64-bit object inside VaryingData is only a small amount of memory overhead, and this is SimpleTimeline, not an especially optimized timeline.
  • Theoretically, it no longer needs to force wrapping its data in an Option. You could construct it with whatever initial value you wanted. However, this is still awkward if it's required to report the event, because the initial value won't have an associated event. Discarding the initial value in forget_before() would also make the code more complicated.
  • What if timelines can only be created in events, and are forbidden from being queried before the creation time? Then we wouldn't need a separate "initial value", and could always return just data (or data + EventHandle).
  • In the absence of query-by-reference, is it worth optimizing by making a query implementation that has a fn(&VaryingData)->Value generic parameter and returns a Value? Or is this something that should be handled on the TimeSteward-API end?
  • Is there a nice way to make several variants of the SimpleTimeline concept? Say, ones that do or don't report EventHandles, ones that do or don't have the query-tracking tree...
@elidupree
Copy link
Owner Author

Conclusions:

  • Don't wrap the data in Option. Also don't have an initial value. If you query it before the first change, panic.
  • Although it's easiest to store every EventHandle, it won't report on them, in the interest of future-proofing the API. (A future implementation might not always have them available.)
  • Due to my decisions on Garbage collection? #32, it will have an optional "destruction time", and at least debug_assert that you can't query it after the destruction time.
  • It's not currently worthwhile to make additional features to work around the lack of query-by-reference.
  • Since modify_simple_timeline() is already in the simple_timeline module, rename it to set() and unmodify_simple_timeline() to unset().
  • It's not currently worthwhile to make several variants of SimpleTimeline. If we make variants in the future, it might involve renaming the current struct, but that's relatively easy to fix in the examples (it would be a simple find-and-replace, or a type alias, or use Foo as Bar).

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

No branches or pull requests

1 participant