I have the following method I came across in a code review. Inside the loop Resharper is telling me that if (narrativefound == false)
is incorrect becuase narrativeFound
is always true. I don't think this is the case, because in order to set narrativeFound
to true it has to pass the conditional string compare first, so how can it always be true? Am I missing something? Is this a bug in Resharper or in our code?
public Chassis GetChassisForElcomp(SPPA.Domain.ChassisData.Chassis asMaintained, SPPA.Domain.ChassisData.Chassis newChassis)
{
Chassis c = asMaintained;
List<Narrative> newNarrativeList = new List<Narrative>();
foreach (Narrative newNarrative in newChassis.Narratives)
{
bool narrativefound = false;
foreach (Narrative orig in asMaintained.Narratives)
{
if (string.Compare(orig.PCode, newNarrative.PCode) ==0 )
{
narrativefound = true;
if (newNarrative.NarrativeValue.Trim().Length != 0)
开发者_如何转开发 {
orig.NarrativeValue = newNarrative.NarrativeValue;
newNarrativeList.Add(orig);
}
break;
}
if (narrativefound == false)
{
newNarrativeList.Add(newNarrative);
}
}
}
c.SalesCodes = newChassis.SalesCodes;
c.Narratives = newNarrativeList;
return c;
}
The variable narrativefound
will never be true when control reaches that statement:
narrativefound = true;
// ...
break; // This causes control to break out of the loop.
I think Resharper is trying to tell you that the condition narrativefound == false
will always be true.
You don't need the narrativeFound
variable at all. In the scope where you set it true, you break from the foreach
loop. If you don't set it to true, you don't break, and you add the newNarrative
to the newNarrativeList
.
So, this could be rewritten as
foreach (Narrative newNarrative in newChassis.Narratives)
{
foreach (Narrative orig in asMaintained.Narratives)
{
if (string.Compare(orig.PCode, newNarrative.PCode) == 0)
{
if (newNarrative.NarrativeValue.Trim().Length != 0)
{
orig.NarrativeValue = newNarrative.NarrativeValue;
newNarrativeList.Add(orig);
}
break;
}
newNarrativeList.Add(newNarrative);
}
}
It's a bug in your code.
foreach (Narrative newNarrative in newChassis.Narratives)
{
bool narrativefound = false;
foreach (Narrative orig in asMaintained.Narratives)
{
if (string.Compare(orig.PCode, newNarrative.PCode) ==0 )
{
narrativefound = true;
if (newNarrative.NarrativeValue.Trim().Length != 0)
{
orig.NarrativeValue = newNarrative.NarrativeValue;
newNarrativeList.Add(orig);
}
// narrativeFound == true, but now we exit the for loop
break;
}
// narrativeFound is always false here. The test is redundant
if (narrativefound == false)
{
newNarrativeList.Add(newNarrative);
}
}
}
R# is correct because if you turn narrativefound to true you are breaking out of the foreach right after you set it.
I believe it is telling you that becuase IF narriativefound is set to true then the for loop is exited (break;). So if the if (narriativefound == false) is evaluated it will always have a value of false.
精彩评论