Coil API Refactor (Part II -- simplify ReportCoilSelection)#11517
Coil API Refactor (Part II -- simplify ReportCoilSelection)#11517
Conversation
amirroth
added
Refactoring
|
|
|
|
amirroth
left a comment
amirroth
left a comment
There was a problem hiding this comment.
Maybe the simplest PR I've ever had. Certainly top five.
| if (this->isCoilReportObject) { | ||
| state.dataRptCoilSelection->coilSelectionReportObj->setCoilEntAirTemp( | ||
| state, this->compName, this->compType, this->autoSizedValue, this->curSysNum, this->curZoneEqNum); | ||
| ReportCoilSelection::setCoilEntAirTemp( |
There was a problem hiding this comment.
This is basically the only type of change in the entire PR. A lot of search-replace and letting the compiler flag the remaining changes that need to be performed by hand.
| #endif // GET_OUT | ||
| }; | ||
|
|
||
| #ifdef GET_OUT |
There was a problem hiding this comment.
Eliminated these while I was at it.
|
|
||
| extern const std::array<bool, (int)CoilType::Num> coilTypeIsHeatPump; | ||
|
|
||
| #ifdef GET_OUT |
| HVAC::CoilType heatCoilType = HVAC::CoilType::Invalid; | ||
|
|
||
| HVAC::CoilType coilType = HVAC::CoilType::Invalid; | ||
| int coilReportNum = -1; |
There was a problem hiding this comment.
Every coil now has a coilReportNum variable which is assigned when the coil is first created.
| heatingCoil.heatCoilType = HVAC::CoilType::HeatingElectric; | ||
| heatingCoil.coilType = HVAC::CoilType::HeatingElectric; | ||
|
|
||
| heatingCoil.coilReportNum = ReportCoilSelection::getReportIndex(state, heatingCoil.Name, heatingCoil.coilType); |
There was a problem hiding this comment.
Create coil, assign coilReportNum.
| struct ReportCoilSelectionData : BaseGlobalStruct | ||
| { | ||
|
|
||
| std::unique_ptr<ReportCoilSelection> coilSelectionReportObj; |
There was a problem hiding this comment.
There is no need for this embedded object just to make the module interface "object-oriented". Object-orientation is in quotes because it wasn't even being applied to the correct object, it was being applied to the container object rather than the individual coilReport objects, i.e., coilContainerObject->setCoilData(coilNum, data); instead of coilReportObject->setCoilData(data);. Anyways, I made this into a simple procedural API.
Is something still wrong with the Ubuntu runner? I don't understand how this type of PR can generate these types of diffs. |
|
|
|
|
I had a theory this morning, so I reran regressions a couple times. But that theory didn't pan out. I'll keep hunting. The latest rerun + local testing is showing that some of the table diffs are real. |
I'm more inclined to believe the MacOS diffs (which have two kinds of diffs including table big diffs) in 14 files than the Ubuntu diffs (which have 16 kinds of diffs in 400 files). |
|
Late edit:
The latest Ubuntu regressions show this as well. |
|
|
Thanks. I will focus on those. |
3 participants
Second part of the Coil API refactor. This part simplifies ReportCoilSelection which was unnecessarily "object-oriented".