get rid of g_axiom_evaluators #235
Conversation
| @@ -108,4 +108,8 @@ void DelegatingTask::convert_ancestor_state_values( | |||
| parent->convert_ancestor_state_values(values, ancestor_task); | |||
| convert_state_values_from_parent(values); | |||
| } | |||
|
|
|||
| AxiomEvaluator &DelegatingTask::get_axiom_evaluator() const { | |||
| return parent->get_axiom_evaluator(); | |||
There was a problem hiding this comment.
Perhaps we need to discuss this design more. There is a difference between the other delegation methods and this one in the sense that the other ones delegate the default but can override the behavior because we want to be able to change the behavior. In contrast, with axioms evaluators there is no conceptual reason to override the behavior; the axiom evaluator should always correctly implement the axioms of the tasks, which are defined by the other methods.
So we're now adding a new responsibility to the tasks to make sure that this happens (rather than having the methods that return the axioms and the method that returns the axiom evaluator behave inconsistently).
I understand that there can be a reason here to delegate for efficiency reasons, but I think correctness would be a better default than efficiency. And I think it needs to be clearer (from the documentation or whatever) in which scenarios one would/would not override this method and why.
| shared_ptr<AbstractTask> get_default_value_axioms_task_if_needed( | ||
| const shared_ptr<AbstractTask> &task, | ||
| AxiomHandlingType axioms) { | ||
| TaskProxy proxy(*task); | ||
| if (task_properties::has_axioms(proxy)) { | ||
| return make_shared<tasks::DefaultValueAxiomsTask>( | ||
| DefaultValueAxiomsTask(task, axioms)); | ||
| return make_shared<tasks::DefaultValueAxiomsTask>(task, axioms); |
There was a problem hiding this comment.
I don't understand this class well enough to review this part of the pull request.
No description provided.