Reactor: ensure consistent vector lengths in C++ layer#756
Reactor: ensure consistent vector lengths in C++ layer#756speth merged 2 commits intoCantera:masterfrom
Conversation
speth
left a comment
speth
left a comment
There was a problem hiding this comment.
I see the issue with the keeping the length of m_advancelimits consistent across different derived classes (including user-created ones). I wanted to propose an alternative solution, which is to simply resize it to the correct size in the few locations where it is used (all of which are within the Reactor base class). See bd58afa for the implementation of this option.
I'm 👍 on making the related methods non-virtual.
|
@speth ... resizing at a later time is a very good suggestion! Not having to resize in overloaded functions is great. Along those lines, I am suggesting to not resize |
speth
left a comment
speth
left a comment
There was a problem hiding this comment.
This looks good to me. I only have a couple of minor suggestions. In addition to these, can you squash the third commit into the first, since it reverts a number of the changes made in the first commit?
- Keep m_advancelimit length at zero unless it is used
ischoegl
force-pushed
the
consistent-limit-length
branch
from
b4d0cce to
52e7920
Compare
Codecov Report
@@ Coverage Diff @@
## master #756 +/- ##
==========================================
+ Coverage 70.83% 70.84% +<.01%
==========================================
Files 372 372
Lines 43720 43732 +12
==========================================
+ Hits 30971 30982 +11
- Misses 12749 12750 +1
Continue to review full report at Codecov.
|
|
Done (and squashed). |
2 participants
Changes proposed in this pull request
Reactor::m_advancelimits.size()isReactor::m_nvm_advancelimits.size()<m_nvin custom additions