Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce Optional_string usage #9232

Merged
merged 14 commits into from
Jan 19, 2022
Merged

Reduce Optional_string usage #9232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8183874
Get the Optional_strings in Fans.cc and .hh
jasondegraw Jan 10, 2022
83efd71
Minor reformat
jasondegraw Jan 10, 2022
bd5a9b5
Remove some optional strings in DXCoils
jasondegraw Jan 10, 2022
b56833f
More work on optional strings
jasondegraw Jan 10, 2022
47da64f
Remover misc. string optionals
jasondegraw Jan 11, 2022
fe06238
Remove optional strings in output
jasondegraw Jan 11, 2022
8f40a07
Merge branch 'optional-string' of https://github.com/NREL/EnergyPlus …
jasondegraw Jan 11, 2022
40e1996
Wrap up a few more of the easier cases
jasondegraw Jan 12, 2022
06eedf8
Merge branch 'develop' into optional-string
jasondegraw Jan 12, 2022
788b2db
clang format changes
jasondegraw Jan 12, 2022
6fdd943
More format fixes
jasondegraw Jan 12, 2022
5c07508
Change one string to string_view
jasondegraw Jan 13, 2022
04503db
Correct conversion error
jasondegraw Jan 13, 2022
4a0936a
Merge develop; resolve conflicts
Myoldmopar Jan 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EnergyPlus/AirLoopHVACDOAS.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ namespace AirLoopHVACDOAS {
}
} else if (SELECT_CASE_var == "FAN:COMPONENTMODEL") {
thisDOAS.m_FanTypeNum = SimAirServingZones::CompType::Fan_ComponentModel;
Fans::GetFanIndex(state, CompName, thisDOAS.m_FanIndex, errorsFound, ObjexxFCL::Optional_string_const());
Fans::GetFanIndex(state, CompName, thisDOAS.m_FanIndex, errorsFound);
thisDOAS.FanName = CompName;
if (CompNum == 1) {
thisDOAS.FanBlowTroughFlag = true;
Expand Down
4 changes: 2 additions & 2 deletions src/EnergyPlus/AirflowNetworkBalanceManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ namespace AirflowNetworkBalanceManager {
// This breaks the component model, need to fix
bool fanErrorFound = false;
int fanIndex;
GetFanIndex(state, thisObjectName, fanIndex, fanErrorFound, ObjexxFCL::Optional_string_const());
GetFanIndex(state, thisObjectName, fanIndex, fanErrorFound);
if (fanErrorFound) {
ShowSevereError(state,
format(RoutineName) + ": " + CurrentModuleObject + " = " + thisObjectName +
Expand Down Expand Up @@ -1388,7 +1388,7 @@ namespace AirflowNetworkBalanceManager {

} else {

GetFanIndex(state, fan_name, fanIndex, FanErrorFound, ObjexxFCL::Optional_string_const());
GetFanIndex(state, fan_name, fanIndex, FanErrorFound);

if (FanErrorFound) {
ShowSevereError(state, "...occurs in " + CurrentModuleObject + " = " + state.dataAirflowNetwork->DisSysCompCVFData(i).name);
Expand Down
26 changes: 14 additions & 12 deletions src/EnergyPlus/BranchNodeConnections.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ void RegisterNodeConnection(EnergyPlusData &state,
NodeInputManager::CompFluidStream const FluidStream, // Count on Fluid Streams
bool const IsParent, // True when node is a parent node
bool &errFlag, // Will be True if errors already detected or if errors found here
Optional_string_const InputFieldName // Input Field Name
std::string_view const InputFieldName // Input Field Name
)
{

Expand Down Expand Up @@ -153,7 +153,7 @@ void RegisterNodeConnection(EnergyPlusData &state,
}

if (has_prefixi(ObjectType, "AirTerminal:")) {
if (present(InputFieldName)) {
if (!InputFieldName.empty()) {
++state.dataBranchNodeConnections->NumOfAirTerminalNodes;
if (state.dataBranchNodeConnections->NumOfAirTerminalNodes > 1 &&
state.dataBranchNodeConnections->NumOfAirTerminalNodes > state.dataBranchNodeConnections->MaxNumOfAirTerminalNodes) {
Expand All @@ -172,7 +172,7 @@ void RegisterNodeConnection(EnergyPlusData &state,
if (Found != 0) { // Nodename already used
ShowSevereError(state, fmt::format("{}{}=\"{}\" node name duplicated", RoutineName, ObjectType, ObjectName));
ShowContinueError(state, "NodeName=\"" + std::string{NodeName} + "\", entered as type=" + std::string{ConnectionType});
ShowContinueError(state, "In Field=" + InputFieldName());
ShowContinueError(state, fmt::format("In Field={}", InputFieldName));
ShowContinueError(state,
"Already used in " + state.dataBranchNodeConnections->AirTerminalNodeConnections(Found).ObjectType + "=\"" +
state.dataBranchNodeConnections->AirTerminalNodeConnections(Found).ObjectName + "\".");
Expand Down Expand Up @@ -1283,13 +1283,13 @@ void GetChildrenData(EnergyPlusData &state,
}

void SetUpCompSets(EnergyPlusData &state,
std::string_view ParentType, // Parent Object Type
std::string_view ParentName, // Parent Object Name
std::string_view CompType, // Component Type
std::string_view CompName, // Component Name
std::string_view InletNode, // Inlet Node Name
std::string_view OutletNode, // Outlet Node Name
Optional_string_const Description // Description
std::string_view ParentType, // Parent Object Type
std::string_view ParentName, // Parent Object Name
std::string_view CompType, // Component Type
std::string_view CompName, // Component Name
std::string_view InletNode, // Inlet Node Name
std::string_view OutletNode, // Outlet Node Name
std::string_view const Description // Description
)
{

Expand Down Expand Up @@ -1348,7 +1348,9 @@ void SetUpCompSets(EnergyPlusData &state,
// Assume this is a further definition for this compset
state.dataBranchNodeConnections->CompSets(Count).ParentCType = ParentTypeUC;
state.dataBranchNodeConnections->CompSets(Count).ParentCName = ParentName;
if (present(Description)) state.dataBranchNodeConnections->CompSets(Count).Description = Description;
if (!Description.empty()) {
state.dataBranchNodeConnections->CompSets(Count).Description = Description;
}
Found = Count;
break;
}
Expand Down Expand Up @@ -1458,7 +1460,7 @@ void SetUpCompSets(EnergyPlusData &state,
UtilityRoutines::MakeUPPERCase(InletNode); // TODO: Fix this....
state.dataBranchNodeConnections->CompSets(state.dataBranchNodeConnections->NumCompSets).OutletNodeName =
UtilityRoutines::MakeUPPERCase(OutletNode); // TODO: Fix this....
if (present(Description)) {
if (!Description.empty()) {
state.dataBranchNodeConnections->CompSets(state.dataBranchNodeConnections->NumCompSets).Description = Description;
} else {
state.dataBranchNodeConnections->CompSets(state.dataBranchNodeConnections->NumCompSets).Description = "UNDEFINED";
Expand Down
16 changes: 8 additions & 8 deletions src/EnergyPlus/BranchNodeConnections.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ namespace BranchNodeConnections {
NodeInputManager::CompFluidStream FluidStream, // Count on Fluid Streams
bool IsParent, // True when node is a parent node
bool &errFlag, // Will be True if errors already detected or if errors found here
Optional_string_const InputFieldName = _ // Input Field Name
std::string_view const InputFieldName = {} // Input Field Name
);

void OverrideNodeConnectionType(EnergyPlusData &state,
Expand Down Expand Up @@ -141,13 +141,13 @@ namespace BranchNodeConnections {
bool &ErrorsFound);

void SetUpCompSets(EnergyPlusData &state,
std::string_view ParentType, // Parent Object Type
std::string_view ParentName, // Parent Object Name
std::string_view CompType, // Component Type
std::string_view CompName, // Component Name
std::string_view InletNode, // Inlet Node Name
std::string_view OutletNode, // Outlet Node Name
Optional_string_const Description = _ // Description
std::string_view ParentType, // Parent Object Type
std::string_view ParentName, // Parent Object Name
std::string_view CompType, // Component Type
std::string_view CompName, // Component Name
std::string_view InletNode, // Inlet Node Name
std::string_view OutletNode, // Outlet Node Name
std::string_view const Description = {} // Description
);

void TestInletOutletNodes(EnergyPlusData &state, bool &ErrorsFound);
Expand Down
27 changes: 12 additions & 15 deletions src/EnergyPlus/DXCoils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15523,7 +15523,7 @@ void GetFanIndexForTwoSpeedCoil(
if (state.dataAirSystemsData->PrimaryAirSystems(FoundAirSysNum).Branch(FoundBranch).Comp(CompNum).CompType_Num ==
SimAirServingZones::CompType::Fan_Simple_VAV) {
SupplyFanName = state.dataAirSystemsData->PrimaryAirSystems(FoundAirSysNum).Branch(FoundBranch).Comp(CompNum).Name;
Fans::GetFanIndex(state, SupplyFanName, SupplyFanIndex, ErrorsFound, ObjexxFCL::Optional_string_const());
Fans::GetFanIndex(state, SupplyFanName, SupplyFanIndex, ErrorsFound);
SupplyFan_TypeNum = DataHVACGlobals::FanType_SimpleVAV;
break;
// these are specified in SimAirServingZones and need to be moved to a Data* file. UnitarySystem=19
Expand Down Expand Up @@ -15687,8 +15687,8 @@ void GetDXCoilIndex(EnergyPlusData &state,
std::string const &DXCoilName,
int &DXCoilIndex,
bool &ErrorsFound,
Optional_string_const ThisObjectType,
Optional_bool_const SuppressWarning)
std::string_view const ThisObjectType,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a good example of the easier changes. The argument is only used for errors, so the argument type is changed first.

bool const SuppressWarning)
{

// SUBROUTINE INFORMATION:
Expand All @@ -15706,11 +15706,10 @@ void GetDXCoilIndex(EnergyPlusData &state,

DXCoilIndex = UtilityRoutines::FindItemInList(DXCoilName, state.dataDXCoils->DXCoil);
if (DXCoilIndex == 0) {
if (present(SuppressWarning)) {
if (!SuppressWarning) {
// No warning printed if only searching for the existence of a DX Coil
} else {
if (present(ThisObjectType)) {
ShowSevereError(state, ThisObjectType + ", GetDXCoilIndex: DX Coil not found=" + DXCoilName);
if (!ThisObjectType.empty()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the call to present is changed to use the empty method.

ShowSevereError(state, fmt::format("{}, GetDXCoilIndex: DX Coil not found={}", ThisObjectType, DXCoilName));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And finally the string handling is changed to something compatible with string_view.

} else {
ShowSevereError(state, "GetDXCoilIndex: DX Coil not found=" + DXCoilName);
}
Expand All @@ -15720,7 +15719,7 @@ void GetDXCoilIndex(EnergyPlusData &state,
}

std::string
GetDXCoilName(EnergyPlusData &state, int &DXCoilIndex, bool &ErrorsFound, Optional_string_const ThisObjectType, Optional_bool_const SuppressWarning)
GetDXCoilName(EnergyPlusData &state, int &DXCoilIndex, bool &ErrorsFound, std::string_view const ThisObjectType, bool const SuppressWarning)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions like this should not be necessary. This should just be state.dataDXCoils->DXCoils(DXCoilNum).Name. The error checking and defaulting should not be necessary. You should not need to call a function to grab a field of an object whose index is known. I understand that this has nothing to do with this PR, but this is as good a place as any for me to hang this comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions should hopefully only get called during the input phase, but (a) good point and (b) I agree.

{

// SUBROUTINE INFORMATION:
Expand All @@ -15737,20 +15736,18 @@ GetDXCoilName(EnergyPlusData &state, int &DXCoilIndex, bool &ErrorsFound, Option
}

if (DXCoilIndex == 0) {
if (present(SuppressWarning)) {
if (!SuppressWarning) {
// No warning printed if only searching for the existence of a DX Coil
} else {
if (present(ThisObjectType)) {
ShowSevereError(state, ThisObjectType + ", GetDXCoilIndex: DX Coil not found ");
if (!ThisObjectType.empty()) {
ShowSevereError(state, fmt::format("{}, GetDXCoilIndex: DX Coil not found ", ThisObjectType));
} else {
ShowSevereError(state, "GetDXCoilIndex: DX Coil not found ");
}
}
ErrorsFound = true;
return " ";
return " "; // This does not seem great

} else {

return state.dataDXCoils->DXCoil(DXCoilIndex).Name;
}
}
Expand Down Expand Up @@ -18395,7 +18392,7 @@ void CalcVRFCoilCapModFac(EnergyPlusData &state,
if (present(CoilIndex)) {
CoilNum = CoilIndex;
} else {
GetDXCoilIndex(state, CoilName, CoilNum, ErrorsFound, ObjexxFCL::Optional_string_const(), ObjexxFCL::Optional_bool_const());
GetDXCoilIndex(state, CoilName, CoilNum, ErrorsFound, {}, true);
}

BFC_rate = state.dataDXCoils->DXCoil(CoilNum).RateBFVRFIUEvap;
Expand Down
6 changes: 3 additions & 3 deletions src/EnergyPlus/DXCoils.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,11 +708,11 @@ namespace DXCoils {
std::string const &DXCoilName,
int &DXCoilIndex,
bool &ErrorsFound,
Optional_string_const ThisObjectType,
Optional_bool_const SuppressWarning);
std::string_view const ThisObjectType = {},
bool const SuppressWarning = false);

std::string GetDXCoilName(
EnergyPlusData &state, int &DXCoilIndex, bool &ErrorsFound, Optional_string_const ThisObjectType, Optional_bool_const SuppressWarning);
EnergyPlusData &state, int &DXCoilIndex, bool &ErrorsFound, std::string_view const ThisObjectType = {}, bool const SuppressWarning = false);

Real64 GetCoilCapacity(EnergyPlusData &state,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing.

std::string const &CoilType, // must match coil types in this module
Expand Down
8 changes: 4 additions & 4 deletions src/EnergyPlus/DataSurfaceColors.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ namespace EnergyPlus::DataSurfaceColors {
// other surface reporting.

bool MatchAndSetColorTextString(EnergyPlusData &state,
std::string const &String, // string to be matched
int const SetValue, // value to be used for the color
std::string const &ColorType // for now, must be DXF
std::string const &String, // string to be matched
int const SetValue, // value to be used for the color
std::string_view const ColorType // for now, must be DXF and probably not a string in the future
)
{

Expand Down Expand Up @@ -109,7 +109,7 @@ bool MatchAndSetColorTextString(EnergyPlusData &state,
return true;
}

void SetUpSchemeColors(EnergyPlusData &state, std::string const &SchemeName, Optional_string_const ColorType)
void SetUpSchemeColors(EnergyPlusData &state, std::string const &SchemeName, std::string_view const ColorType)
{

// SUBROUTINE INFORMATION:
Expand Down
8 changes: 4 additions & 4 deletions src/EnergyPlus/DataSurfaceColors.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ namespace DataSurfaceColors {
};

bool MatchAndSetColorTextString(EnergyPlusData &state,
std::string const &String, // string to be matched
int SetValue, // value to be used for the color
std::string const &ColorType // for now, must be DXF
std::string const &String, // string to be matched
int SetValue, // value to be used for the color
std::string_view const ColorType // for now, must be DXF
);

void SetUpSchemeColors(EnergyPlusData &state, std::string const &SchemeName, Optional_string_const ColorType = _);
void SetUpSchemeColors(EnergyPlusData &state, std::string const &SchemeName, std::string_view const ColorType);

} // namespace DataSurfaceColors

Expand Down
41 changes: 18 additions & 23 deletions src/EnergyPlus/DataZoneEquipment.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ void GetZoneEquipmentData(EnergyPlusData &state)

// Using/Aliasing
using BranchNodeConnections::SetUpCompSets;
using NodeInputManager::CheckUniqueNodes;
using NodeInputManager::CheckUniqueNodeNames;
using NodeInputManager::CheckUniqueNodeNumbers;
using NodeInputManager::EndUniqueNodeCheck;
using NodeInputManager::GetNodeNums;
using NodeInputManager::GetOnlySingleNode;
Expand Down Expand Up @@ -321,7 +322,7 @@ void GetZoneEquipmentData(EnergyPlusData &state)
state.dataZoneEquip->GetZoneEquipmentDataErrorsFound = true;
} else {
UniqueNodeError = false;
CheckUniqueNodes(state, cAlphaFields(5), "NodeName", UniqueNodeError, AlphArray(5), _, AlphArray(1));
CheckUniqueNodeNames(state, cAlphaFields(5), UniqueNodeError, AlphArray(5), AlphArray(1));
if (UniqueNodeError) {
// ShowContinueError(state, "Occurs for " + trim( cAlphaFields( 1 ) ) + " = " + trim( AlphArray( 1 ) ) );
state.dataZoneEquip->GetZoneEquipmentDataErrorsFound = true;
Expand Down Expand Up @@ -693,13 +694,11 @@ void GetZoneEquipmentData(EnergyPlusData &state)
for (NodeNum = 1; NodeNum <= NumNodes; ++NodeNum) {
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).InletNode(NodeNum) = NodeNums(NodeNum);
UniqueNodeError = false;
CheckUniqueNodes(state,
"Zone Air Inlet Nodes",
"NodeNumber",
UniqueNodeError,
_,
NodeNums(NodeNum),
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ZoneName);
CheckUniqueNodeNumbers(state,
"Zone Air Inlet Nodes",
UniqueNodeError,
NodeNums(NodeNum),
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ZoneName);
if (UniqueNodeError) {
state.dataZoneEquip->GetZoneEquipmentDataErrorsFound = true;
}
Expand Down Expand Up @@ -741,13 +740,11 @@ void GetZoneEquipmentData(EnergyPlusData &state)
for (NodeNum = 1; NodeNum <= NumNodes; ++NodeNum) {
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ExhaustNode(NodeNum) = NodeNums(NodeNum);
UniqueNodeError = false;
CheckUniqueNodes(state,
"Zone Air Exhaust Nodes",
"NodeNumber",
UniqueNodeError,
_,
NodeNums(NodeNum),
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ZoneName);
CheckUniqueNodeNumbers(state,
"Zone Air Exhaust Nodes",
UniqueNodeError,
NodeNums(NodeNum),
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ZoneName);
if (UniqueNodeError) {
// ShowContinueError(state, "Occurs for Zone = " + trim( AlphArray( 1 ) ) );
state.dataZoneEquip->GetZoneEquipmentDataErrorsFound = true;
Expand Down Expand Up @@ -794,13 +791,11 @@ void GetZoneEquipmentData(EnergyPlusData &state)
for (NodeNum = 1; NodeNum <= NumNodes; ++NodeNum) {
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ReturnNode(NodeNum) = NodeNums(NodeNum);
UniqueNodeError = false;
CheckUniqueNodes(state,
"Zone Return Air Nodes",
"NodeNumber",
UniqueNodeError,
_,
NodeNums(NodeNum),
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ZoneName);
CheckUniqueNodeNumbers(state,
"Zone Return Air Nodes",
UniqueNodeError,
NodeNums(NodeNum),
state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum).ZoneName);
if (UniqueNodeError) {
// ShowContinueError(state, "Occurs for Zone = " + trim( AlphArray( 1 ) ) );
state.dataZoneEquip->GetZoneEquipmentDataErrorsFound = true;
Expand Down
Loading