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

Design a more ergonomic API. #66

Closed
afck opened this issue Jun 19, 2018 · 4 comments · Fixed by #130
Closed

Design a more ergonomic API. #66

afck opened this issue Jun 19, 2018 · 4 comments · Fixed by #130

Comments

@afck
Copy link
Collaborator

afck commented Jun 19, 2018

The current DistAlgorithm trait makes it easy for the caller to forget checking next_message and next_output. It would probably be safer to go back to a design where handle_message directly returns a (#[must_use]?) result that contains the messages and output.

We should also consider moving DistAlgorithm into the tests. Even if we don't, all the algorithms should at least be usable without it, too.

@vkomenda
Copy link
Contributor

Here is the way I think we should change the API. Please feel free to comment. I propose to keep the DistAlgorithm interface in a modified form for internal purposes, making it more automaton-like. At the moment DistAlgorithm already has semantics close to that of an automaton - with Input and Output but without state. I believe that the interface should only have

  1. a step function from inputs (and states, internally) to outputs and states,

  2. types of input value, output value and state.

An input is an incoming message and an optional input value.

An output is a list of outgoing messages, a list of log entries and an optional output value. The caller #[must_use] the output.

The usual type of state will be the set of four values:

a. not initialized (accepting incoming messages and input values),

b. initialized (accepting incoming messages and discarding input values),

c. terminated (discarding everything),

d. error.

The state value is a field in the algorithm struct and is output at each step, so there is no need to have an is_terminated function to check for termination.

I think that keeping errors separate from state as in Result<OutputAndState, Error> would only follow the requirements of external crates such as error_chain and is not a necessity.

That was the easy part. So, what does it mean for the user of the library? If you use any of the algorithm structs directly, you must know the types of its input values, output values, messages and states. The top level API can provide an interface call to the step function, so that using the DistAlgorithm trait is not needed in user code.

@afck
Copy link
Collaborator Author

afck commented Jun 27, 2018

I agree with the plan for the outputs, and I like the idea of replacing multiple booleans (like terminated) with a single state enum, where possible.

An input is an incoming message and an optional input value.

I think we would end up only ever calling input(None, Some(input)) or input(Some(msg), None), so maybe it should be two separate functions.

@afck
Copy link
Collaborator Author

afck commented Jul 4, 2018

Let's also keep in mind the Rust API guidelines whenever we make changes to the API.

@vkomenda
Copy link
Contributor

vkomenda commented Jul 4, 2018

I created an issue for that: #108.

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

Successfully merging a pull request may close this issue.

2 participants