Simplify implementation of DataContext.#1435
Conversation
BenjaminRodenberg
added
the
maintainability
uekerman
left a comment
uekerman
left a comment
There was a problem hiding this comment.
Makes things easier, yes.
Only one doubt below, otherwise good to go.
| std::for_each(_toData.begin(), _toData.end(), [](auto &data) { data->toZero(); }); | ||
| // PRECICE_ASSERT(!hasReadMapping()); // we also need this assertion, because currently we also reset toData from read mappings, if a data context has a read and write mapping! Is this what we want? | ||
| std::for_each(_mappingContexts.begin(), _mappingContexts.end(), [](auto &context) { context.toData->toZero(); }); |
There was a problem hiding this comment.
I think I don't understand the comment here. Can a data context have a read and a write mapping? I would guess no.
And where do we reset toData?
Sth different: maybe it's cleaner if we move the resetting to a MappingContext::resetData?
There was a problem hiding this comment.
I think I don't understand the comment here. Can a data context have a read and a write mapping? I would guess no. And where do we reset toData?
I tried out what happens, if I activate the assertion and it does not get triggered. I thought this was the reason for originally commenting it out, but everything looks fine now: A DataContext either has a read or write mapping, not both at the same time (at least, if we reset data).
I added the assertion in 5a0ff28 and I think we can merge it this way (if the tests run though).
Sth different: maybe it's cleaner if we move the resetting to a
MappingContext::resetData?
Yes. But MappingContext is a struct, so moving something inside MappingContext will cause a bigger refactoring and this is from my perspective out of scope for this PR. Opening an isssue would be useful and I guess it would also be a low-hanging fruit, if somebody wants to get his or her hands dirty with C++ and preCICE.
2 participants
Main changes of this PR
Motivation and additional information
Helpful in scope of #1414
Author's checklist
pre-commithook to prevent dirty commits and usedpre-commit run --allto format old commits.make changelogif there are user-observable changes since the last release. (not needed?)Reviewers' checklist