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

RFC: statemachine.State as an interface instead of a struct #135

Open
schmichael opened this issue Jul 24, 2015 · 1 comment
Open

RFC: statemachine.State as an interface instead of a struct #135

schmichael opened this issue Jul 24, 2015 · 1 comment
Labels

Comments

@schmichael
Copy link
Contributor

Pros to an interface

  • Users can tuck extra metadata into their implementation
  • Manually creating State structs smells funny

Cons to an interface

  • Allows "clever" implementations of methods that should just be getters for immutable values
  • Suggests/forces custom implementations which is extra work for users
  • Custom implementations mean Yet Another Place to Hide State.

Since #133 just merged I'd rather see how far the Task interface gets us than rush and interface-ify everything and create useless complexity.

@schmichael schmichael added the RFC label Jul 24, 2015
@schmichael
Copy link
Contributor Author

Welp, already found an example of where tucking extra metadata into the State would be helpful. Our internal State model is persisted with Created and Modified timestamps. Modified is easy: you just overwrite it with time.Now() every time.

Created however is a piece of State-specific metadata that needs to be roundtripped from StateStore.Load through the statemachine and back to StateStore.Store. Since statemachine.State is a struct, there's nowhere to put that metadata.

There are 2 ways to mitigate this:

  1. Add a StateCreated field to our custom Task implementation and hide the extra metadata there since Task is passed through the statemachine to Load and Store. Yuck! Gross encapsulation/isolation violation.
  2. Simply set the State's Created timestamp to the Task's Created timestamp. Not ideal, but honestly the difference between the two would only represent some implementation specific latency number that's best measured other ways.

Going with 2 for now... may still end up switching State to an interface though.

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

No branches or pull requests

1 participant