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

use non generative api #12

Closed
c-cube opened this issue Dec 28, 2017 · 4 comments
Closed

use non generative api #12

c-cube opened this issue Dec 28, 2017 · 4 comments
Assignees
Milestone

Comments

@c-cube
Copy link
Collaborator

c-cube commented Dec 28, 2017

replace generative functors by mere functors (or modules), and provide a type t which wraps the state.

pros:

  • state is more explicit
  • globals are now embedded in t and can be gc'd
  • more customizable: val create: ?size:int -> ?callback:… -> unit -> t
  • easier to copy/(de)serialize if need be
  • easier to integrate in other type t-based APIs
  • more convenient (no local module necessary)

cons:

  • change API
@c-cube c-cube self-assigned this Dec 28, 2017
@c-cube c-cube added this to the 1.0 milestone Dec 28, 2017
@Gbury
Copy link
Owner

Gbury commented Dec 29, 2017

I don't think that breaking the API is a big problem, as I don't get the felling there are a lot of mSAT users yet.

@timothee-haudebourg
Copy link

I can't use this in my project (yet) because of the generative api. Do you have any idea when the type t will be implemented?

@Gbury
Copy link
Owner

Gbury commented May 25, 2018

There is a pull request ( #11 ) implementing this suggestion. What slows me from merging it is that I'm currently writing my thesis with my main project using mSAT, so I'm not too keen on adapting it right now.

However, it could be possible for me to release the current version of mSAT, and then merge PR #11 . Though you'd have to pin the github repo to get the version with explicit state (at least until the merged version has been tested enough for a release). Would that help you ?

@timothee-haudebourg
Copy link

Yes that would help, thanks. But I didn't realized that it was included in #11, if you need some time I can just get it from this branch.

@c-cube c-cube closed this as completed Feb 14, 2019
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

3 participants