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

Arity specialized branching constructs #23

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

Munksgaard
Copy link
Owner

This is an experiment that introduces arity specialized branching
constructs. It's not meant to be immediately merged, but to serve as a discussion point regarding this alternative library design, and whether it's something we might want.

The idea is to add multiple specialized Offer/Choose constructs
according to arity. That way, instead of chaining lots of Offer's and
having to do sel2().sel2().sel2(), we'd simply have an OfferN with
associated sel1(), sel2(), ..., and selN() methods.

So far I've only introduced Offer3 and Choose3 structs, but they serve to
give us an idea of how the library would end up looking.

Edit: Almost all of the tests and examples will be failing right now, but that's to be expected. Consult the ATM example (examples/atm.rs) to see how it looks from a users perspective.

This is an experiment that introduces arity specialized branching
constructs.

The idea is to add multiple specialized Offer/Choose constructs
according to arity. That way, instead of chaining lots of Offer's and
having to do `sel2().sel2().sel2()`, we'd simply have an `OfferN` with
associated `sel1()`, `sel2()`, ..., and `selN()` methods.

This commit only introduces Offer3 and Choose3 structs, but serves to
give us an idea of how our library would end up looking.
@Munksgaard
Copy link
Owner Author

We'd definitely want some macros to actually define the different constructs and methods instead of doing it manually.

@Munksgaard
Copy link
Owner Author

Pros:

  • More user friendly. The user doesn't need huge nested Offer's and Choose's.
  • Performance. When choosing between n (n < 256) branches, we only have to send one byte, instead of O(log n) bytes or O(n) bytes.

Cons:

  • Less user friendly. The user has to keep track of many different branching constructs. For example, not only does he need to choose between Offer2, Offer3 and so on, but when matching against an offer() call, the matches explicitly mention the arity of the offer call. For example, if c is an Offer3 branch, something like the following is necessary:
match c.offer() {
    Branch3::B1(c) => ...
    Branch3::B2(c) => ...
    Branch3::B3(c) => ...
}
...

If you later change the arity of the branch you have to actually change all of these Branch3s to Branch4s, for example.

  • Library bloat. The library becomes a lot bigger. We can use macros that declare the structs and methods for us to help alleviate this.

@Manishearth
Copy link
Collaborator

I wonder if we can make this work with tuples?

Definitely prettier, but we still need the BranchN constructs.
@Munksgaard
Copy link
Owner Author

Good idea, @Manishearth! Check out c936af4. We still need the BranchN structs as far as I can tell, but we get rid of the multitude of OfferN structs.

@Manishearth
Copy link
Collaborator

We could just impl Dual on all the tuple types with a macro, and then the rest should follow on generic Offer<T> and Choose<T> structs.

@Manishearth
Copy link
Collaborator

Oh, but the enums need to be made no matter what. In that case we shouldn't complicate it with tuples and just macro all the things I guess. The macro for this is pretty simple afaict.

@Munksgaard
Copy link
Owner Author

I actually think I prefer the tupled version over the OfferN version regardless.

@Manishearth
Copy link
Collaborator

SGTM. We can always make OfferN typedefs anyway.

@laumann laumann mentioned this pull request Oct 29, 2015
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 this pull request may close these issues.

2 participants