Cpp check redundant assignment by dareumnam · Pull Request #11441 · NatLabRockies/EnergyPlus

dareumnam · 2026-02-27T15:56:37Z

Pull request overview

Fix cppcheck redundantAssignment warnings
Removed redundant assignments when their values were reassigned before the old one has been used.
Added comments where I made changes beyond simply removing redundant assignments

Description of the purpose of this PR

Pull Request Author

Reviewer

…into cppCheck_redundantAssignment

dareumnam · 2026-02-27T15:58:45Z

src/EnergyPlus/DXCoils.cc

-            } else {
-                S1PLR = 0.0;
-            }
+


S1PLR gets set right after the stage-1 full-load calc, but that value is never used before S1PLR is recomputed in the “Determine run-time fractions” block, line 545 - 550 (develop). We can just drop the first S1PLR = PartLoadRatio or 0.0 assignment (no behavior change, since it was overwritten before use).

dareumnam · 2026-02-27T16:00:14Z

src/EnergyPlus/DXCoils.cc


            // Stage 1
-        } else if (CycRatio > 0.0 || (CycRatio > 0.0 && SingleMode == 1)) {
+        } else if (CycRatio > 0.0 || (CycRatio > 0.0 && SingleMode == 1)) { // cppCheck Redundant Condition flag


If the intent was to include SingleMode in the logic, this might need to be CycRatio > 0.0 || SingleMode == 1 or CycRatio > 0.0 && SingleMode == 1; otherwise it can be simplified to CycRatio > 0.0.

SingleMode (1 = Yes, 0 = No) does operate just like a cycling coil except that at speeds > 1 the cycling is between capacity at speed = x and off. There are other logic conditionals like this in code and I think testing would be required to know for sure but your suggestion of CycRatio > 0.0 may be correct since line 14070 says "Stage 1" and at stage 1 it wouldn't matter what SingleMode was.

if (SpeedNum > 1 && SingleMode == 0) { } else if (CycRatio > 0.0) {

@rraustad Thanks for your input and for confirming the fix.

dareumnam · 2026-02-27T16:32:03Z

src/EnergyPlus/ThermalComfort.cc

                            lineIn = statFile.readLine();
                        }
-                        lineIn = statFile.readLine();
-                        lineAvg = lineIn.data;


The last lineIn = readLine() inside the 7-line skip loop is immediately overwritten by the next readLine() before it’s ever used. Easiest fix is to discard the skipped lines without assigning to lineIn, then read the actual avg line once into lineAvg.

dareumnam · 2026-02-27T16:38:12Z

src/EnergyPlus/ThermalISO15099Calc.cc

        }

        // no unshaded run for now
        NeedUnshadedRun = false;


NeedUnshadedRun is possibly set true a few lines before, but then immediately forced to false before it’s ever used, so the earlier assignments are dead. Either drop the unconditional NeedUnshadedRun = false; (to keep the logic), or remove the previous NeedUnshadedRun = true lines would be the fix. As the comment said "no unshaded run for now", I left NeedUnshadedRun = false and erased two NeedUnshadedRun = true lines.

…into cppCheck_redundantAssignment

dareumnam · 2026-03-06T16:01:10Z

src/EnergyPlus/VariableSpeedCoils.cc

                // Get Water System tank connections
                //  A8, \field Name of Water Storage Tank for Supply
-                cFieldName = "Supply Water Storage Tank Name"; // cAlphaFields(8)
+                // cFieldName = "Supply Water Storage Tank Name"; // cAlphaFields(8)


I avoided re-declaring cFieldName in inner scopes (like in the previous commit) since that caused a mac build issue. Instead, I commented out the two cFieldName assignments that were never used before being overwritten.

Agreed. This is fine to leave like this for reference.

dareumnam · 2026-03-06T18:14:27Z

src/EnergyPlus/WeatherManager.cc

-                        lineIn = statFile.readLine();
-                        lineAvg = lineIn.data;
+                        auto lineAvgIn = statFile.readLine();
+                        lineAvg = lineAvgIn.data;


The last lineIn = statFile.readLine(); inside the 7-line skip loop is immediately overwritten by the next readLine() before it’s ever used. Easiest fix is to discard the skipped lines without assigning to lineIn, then read the actual avg line once into lineAvg.

I believe the intent of this was just to cycle through until they have read enough lines to get to the one they do want. I'll push a tiny cleanup here.

dareumnam · 2026-03-06T18:17:28Z

src/EnergyPlus/VariableSpeedCoils.cc


                cFieldName = "Condenser Type"; // cAlphaFields(6)
                std::string const condenserType = s_ip->getAlphaFieldValue(fields, schemaProps, "condenser_type");
                if ((Util::SameString(condenserType, "AirCooled")) || cFieldName.empty()) {


I also spotted what looks like a bug in this line:
|| cFieldName.empty() in the Condenser Type check probably meant condenserType.empty() I guess, as cFieldName has always have a value due to line 842 (2 lines above).

Yes, this does look like a bug. Feel free to submit an issue and PR separately.

dareumnam · 2026-03-06T18:25:48Z

src/EnergyPlus/DXCoils.cc

            // get current total capacity, SHR, EIR
            if (SpeedRatio >= 1.0) {
                TotCap = TotCapHS;
-                SHR = SHRHS;


Cppcheck is flagging SHR here because it gets assigned in the SpeedRatio >= 1.0 path, then reassigned again later in the UserSHRCurveExists block before the earlier value is actually used. This looks more like a redundant intermediate assignment than a functional issue.
In if (thisDXCoil.UserSHRCurveExists) - else block (line 11817 in this branch), SHR is assigned on all paths, so the warning is about this SHR assignment here being overwritten before use.

…into cppCheck_redundantAssignment

dareumnam added 5 commits February 20, 2026 00:13

redundantAssignment1

5d4a70c

redundantAssignment2

0a492a1

Merge branch 'develop' of https://github.com/NatLabRockies/EnergyPlus …

661672f

…into cppCheck_redundantAssignment

redundantAssignment3

9ec0795

Merge branch 'develop' of https://github.com/NatLabRockies/EnergyPlus …

f5abbd3

…into cppCheck_redundantAssignment

dareumnam added the DoNotPublish Includes changes that shouldn't be reported in the changelog label Feb 27, 2026

dareumnam commented Feb 27, 2026

View reviewed changes

dareumnam added 2 commits March 6, 2026 08:31

fix build errors and redundantAssignment 4

c0a8e48

Merge branch 'develop' of https://github.com/NatLabRockies/EnergyPlus …

18488b9

…into cppCheck_redundantAssignment

dareumnam commented Mar 6, 2026

View reviewed changes

dareumnam and others added 2 commits March 6, 2026 13:21

Merge branch 'develop' of https://github.com/NatLabRockies/EnergyPlus …

5477058

…into cppCheck_redundantAssignment

Merge branch 'develop' into cppCheck_redundantAssignment

e8ab095

mitchute added this to the EnergyPlus 26.1 Bug Fix Freeze milestone Mar 10, 2026

mitchute marked this pull request as ready for review March 10, 2026 00:12

tiny weather file cleanup

344c911

Conversation

dareumnam commented Feb 27, 2026

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

dareumnam Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dareumnam Feb 27, 2026 •

edited

Loading