add full api to engine #50
Conversation
src/engine.rs
Outdated
| } | ||
|
|
||
| impl Engine { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| pub fn new() -> Result<Self> { |
There was a problem hiding this comment.
In future we can change this so that all our new functions return T instead of Result; new functions are expected to always succeed.
src/engine.rs
Outdated
| } | ||
|
|
||
| pub fn add_data(&mut self, data: Value) -> Result<()> { | ||
| self.data.merge(data) | ||
| self.prepared = false; | ||
| self.interpreter.get_data().merge(data) |
There was a problem hiding this comment.
get_data_mut() reads better.
src/engine.rs
Outdated
| // Analyze the modules and determine how statements must be scheduled. | ||
| let analyzer = Analyzer::new(); | ||
| let schedule = analyzer.analyze(&self.modules)?; | ||
| pub fn prepare_for_eval(&mut self, enable_tracing: bool) -> Result<()> { |
There was a problem hiding this comment.
Does this function need to be public?
There was a problem hiding this comment.
I think not, previously it was necessary for the end-user to explicitly calls this function and now it's done automatically behind the hood when calling eval_* functions of the engine. I don't see a valid reason for the end-user to call it.
src/interpreter.rs
Outdated
| @@ -86,16 +86,15 @@ struct LoopExpr { | |||
| } | |||
|
|
|||
| impl Interpreter { | |||
| pub fn new(modules: &[Ref<Module>]) -> Result<Interpreter> { | |||
| pub fn new() -> Result<Interpreter> { | |||
There was a problem hiding this comment.
In future we can change this so that Interpreter is returned instead of Result
src/interpreter.rs
Outdated
| pub fn set_modules(&mut self, modules: &[Ref<Module>]) { | ||
| self.modules = modules.to_vec(); | ||
| } | ||
|
|
||
| pub fn get_modules(&mut self) -> &mut Vec<Ref<Module>> { |
There was a problem hiding this comment.
get_modules_mut is a better name since a mutable reference is returned.
src/interpreter.rs
Outdated
| self.init_data = init_data; | ||
| } | ||
|
|
||
| pub fn get_init_data(&mut self) -> &mut Value { |
There was a problem hiding this comment.
get_init_data_mut is a better name since a mutable reference is returned.
Alternatively we could have a merge_init_data(&mut self, v : Value) that performs a merge of init data. That way we don't need to return mutable references.
There was a problem hiding this comment.
Apparently I don't use get_init_data anylonger, I forgot to delete it.
There is a set_init_data(&mut self, v : Value) that does what you expect?
eric-therond
force-pushed
the
moveapitoengine
branch
from
40c408e to
348cb82
Compare
|
Can you explain why? I don't see anything in the |
eric-therond
force-pushed
the
moveapitoengine
branch
from
348cb82 to
5f8c613
Compare
Signed-off-by: eric-therond <eric.therond.fr@gmail.com>
eric-therond
force-pushed
the
moveapitoengine
branch
from
a897c46 to
3732a03
Compare
I'm thinking a rule that depends on input values like below would need to be reevaluated when input changes |
Reading the code, currently we don't depend on input in prepare_for_eval. I had a different re-evaluation scheme in mind when I made the earlier remark. |
following #36
to move the rest of the user-API from interpreter to engine