Add a first test case for reservoir coupling #5893
Conversation
hakonhagland
force-pushed
the
tstep_test2
branch
2 times, most recently
from
a347e13 to
e555c94
Compare
|
jenkins build this please |
hakonhagland
force-pushed
the
tstep_test2
branch
2 times, most recently
from
7189b9c to
6e50ec9
Compare
|
jenkins build this please |
tests/rescoup/test_chopstep.cpp
Outdated
| rc_master.resizeNextReportDates(2); | ||
| } | ||
|
|
||
| void setSlaveData(int slave_number, double slave_start_date, double report_time_step_size) |
There was a problem hiding this comment.
Maybe rename this to make clear what is set?
hakonhagland
force-pushed
the
tstep_test2
branch
from
6e50ec9 to
dd24a05
Compare
|
@blattms Thanks for the review. I have added two commits to address the comments. |
|
jenkins build this please |
tests/rescoup/test_chopstep.cpp
Outdated
| // Sets the slave start date and the next report time offset for the given slave number | ||
| // NOTE: The method name is kept short as setSlaveDate() instead of a longer descriptive name | ||
| // like setSlaveStartDateAndNextReportTimeOffset() because the method is called multiple times | ||
| // in the test cases and the shorter name makes the test cases more readable. | ||
| void setSlaveData(int slave_number, double slave_start_date, double report_time_step_size) |
There was a problem hiding this comment.
I kind of disagree. The reason that I asked for a name change is such that fellow developers do not need to take a look at the documentation to understand what is going on. IMHO a short name that does not reveal anything does not make code more readable.
There was a problem hiding this comment.
make a struct, document the struct members, pass an instance of that struct, everybody wins.
There was a problem hiding this comment.
The reason that I asked for a name change is such that fellow developers do not need to take a look at the documentation to understand what is going on.
@blattms In general I would agree, but in this case the function is called multiple times and the long name could become distracting (reducing readability).
|
jenkins build this please |
|
Thanks. Yes that is a long function name. |
|
This breaks the post-commit build. Please fix immediately or back out the PR. |
|
i'm on it |
Builds on #5620.