Conversation
src/EnergyPlus/HeatBalanceManager.cc
Outdated
| if (Util::IsNameEmpty(state, state.dataIPShortCut->cAlphaArgs(1), state.dataHeatBalMgr->CurrentModuleObject, ErrorsFound)) { | ||
| if (state.dataIPShortCut->cAlphaArgs(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Don't need this check because:
Zone,
A1 , \field Name
\required-field
This test:
Zone,
, !- Name
0, !- Direction of Relative North {deg}
gives this error from the InputProcessor:
** Severe ** <root>[Zone] - Object name is required and cannot be blank or whitespace
There was a problem hiding this comment.
Thanks. I'll remove this check.
src/EnergyPlus/NodeInputManager.cc
Outdated
| if (Util::IsNameEmpty(state, cAlphas(1), CurrentModuleObject, localErrorsFound)) { | ||
| if (cAlphas(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
NodeList,
A1 , \field Name
\required-field
There was a problem hiding this comment.
Thanks. I'll remove this check.
src/EnergyPlus/Photovoltaics.cc
Outdated
| if (Util::IsNameEmpty(state, s_ipsc->cAlphaArgs(1), s_ipsc->cCurrentModuleObject, ErrorsFound)) { | ||
| if (s_ipsc->cAlphaArgs(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This one should be required (because it is checked as such) but it's not so this is OK. I suspect all object names should be required-field but there may be exceptions. i.e., be careful which one of these checks get deleted. I always check the idd when adding/removing input checks.
PhotovoltaicPerformance:Simple,
A1 , \field Name
\reference PVModules
There was a problem hiding this comment.
The next 2 are OK as-is (Name is not a required field).
mitchute
left a comment
There was a problem hiding this comment.
I checked all objects where these were used against the IDD. Every one except the PhotovoltaicPerformance objects that I noted are required in the IDD, so these checks should not be needed.
src/EnergyPlus/HeatBalanceManager.cc
Outdated
| if (Util::IsNameEmpty(state, state.dataIPShortCut->cAlphaArgs(1), state.dataHeatBalMgr->CurrentModuleObject, ErrorsFound)) { | ||
| if (state.dataIPShortCut->cAlphaArgs(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Thanks. I'll remove this check.
src/EnergyPlus/NodeInputManager.cc
Outdated
| if (Util::IsNameEmpty(state, cAlphas(1), CurrentModuleObject, localErrorsFound)) { | ||
| if (cAlphas(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Thanks. I'll remove this check.
|
|
||
| ErrorObjectHeader eoh{routineName, s_ipsc->cCurrentModuleObject, s_ipsc->cAlphaArgs(1)}; | ||
|
|
||
| if (Util::IsNameEmpty(state, s_ipsc->cAlphaArgs(1), s_ipsc->cCurrentModuleObject, ErrorsFound)) { |
There was a problem hiding this comment.
Adding \required-field for the PhotovoltaicPerformance:Simple name. OK to remove after that.
| s_ipsc->cNumericFieldNames); | ||
|
|
||
| ErrorObjectHeader eoh{routineName, s_ipsc->cCurrentModuleObject, s_ipsc->cAlphaArgs(1)}; | ||
| if (Util::IsNameEmpty(state, s_ipsc->cAlphaArgs(1), s_ipsc->cCurrentModuleObject, ErrorsFound)) { |
There was a problem hiding this comment.
PhotovoltaicPerformance:EquivalentOne-Diode name should be required, right?
| s_ipsc->lAlphaFieldBlanks, | ||
| s_ipsc->cAlphaFieldNames, | ||
| s_ipsc->cNumericFieldNames); | ||
| if (Util::IsNameEmpty(state, s_ipsc->cAlphaArgs(1), s_ipsc->cCurrentModuleObject, ErrorsFound)) { |
There was a problem hiding this comment.
PhotovoltaicPerformance:Sandia name should be required, right?
…bject names required in the IDD
| \memo which surfaces convert incident solar radiation to electricity and need not specify | ||
| \memo arrays of specific modules. | ||
| A1 , \field Name | ||
| \required-field |
There was a problem hiding this comment.
I went ahead and made these required since I can't see why they should not be.
| if (Util::IsNameEmpty(state, state.dataIPShortCut->cAlphaArgs(1), CurrentModuleObject, ErrorsFound)) { | ||
| if (state.dataIPShortCut->cAlphaArgs(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
PlantEquipmentOperationSchemes,
\min-fields 4
A1 , \field Name
\required-field
There was a problem hiding this comment.
Thanks for looking this over @rraustad. I'll clean out these last few.
| if (Util::IsNameEmpty(state, state.dataIPShortCut->cAlphaArgs(1), CurrentModuleObject, ErrorsFound)) { | ||
| if (state.dataIPShortCut->cAlphaArgs(1).empty()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
CondenserEquipmentOperationSchemes,
\min-fields 4
A1 , \field Name
\required-field
src/EnergyPlus/ThermalChimney.cc
Outdated
|
|
||
| if (Util::IsNameEmpty(state, state.dataIPShortCut->cAlphaArgs(1), cCurrentModuleObject, ErrorsFound)) { | ||
| if (state.dataIPShortCut->cAlphaArgs(1).empty()) { | ||
| continue; |
There was a problem hiding this comment.
ZoneThermalChimney,
\min-fields 10
A1, \field Name
\required-field
| continue; | ||
| } | ||
| Alphas(1) = Alphas(1) + "--dup"; | ||
| } |
There was a problem hiding this comment.
I wonder why a Branch name would be protected like this for a duplicate name?
Branch,
A1, \field Name
\required-field
There was a problem hiding this comment.
Fair question. I'll take another look at it.
There was a problem hiding this comment.
This could be a pre Input Processor unit test? But there is only 1 Branch in this unit test? The only concern I have is whether using the same branch name in different Branch objects is caught. I would suspect it is but can't find that in code or unit tests.
There was a problem hiding this comment.
As far as I can tell, this was added about 10 years ago, by me...... #5575. No idea on why I did it or where I would have gotten that from. Unless we can point to some reason to keep it, I think it's safe to remove at this point.
|
Thanks for the careful review @rraustad. This looks ready. Merging. |
Pull request overview
Reviewing PR #11411 prompted me to look at UtilityRoutines, where it appears we have a number of obsolete functions. This PR cleans these out in lieu of better approaches.
VerifyNameI think nearly all of this is handled already by the epJSON schema, and whatever is left can be handled by the.empty()string method.Description of the purpose of this PR
Pull Request Author
Reviewer