Is there a tool that can detect the following situation

  • Thread starter Thread starter Dave Harris
  • Start date Start date
D

Dave Harris

In the following code, the output is 1 instead of 7. I understand that the
generic list<> is generating a temporary copy of PointD, and then the Set()
call works on the temporary and so the result is lost. It's just subtle
enough though, that unless you are paying really good attention, it's easy to
miss. FXCop doesn't seem to catch it, not does resharper. Aside from
changing the struct to a class, other ideas? Thanks.

namespace ConsoleApplication1
{

public struct PointD
{
public double X;
public double Y;
public double Z;

public PointD(double x, double y, double z)
{
X = x;
Y = y;
Z = z;
}

public void Set(double x, double y, double z)
{
X = x;
Y = y;
Z = z;
}
}

public class M
{
static public void Main()
{
List<PointD> ps = new List<PointD>();
ps.Add(new PointD(1, 2, 3));
ps.Add(new PointD(4, 5, 6));

ps[0].Set(7, 8, 9);

Console.WriteLine(ps[0].X);
}
}
}
 
Hello Dave,
In the following code, the output is 1 instead of 7. I understand
that the generic list<> is generating a temporary copy of PointD, and
then the Set() call works on the temporary and so the result is lost.
It's just subtle enough though, that unless you are paying really good
attention, it's easy to miss. FXCop doesn't seem to catch it, not
does resharper. Aside from changing the struct to a class, other
ideas? Thanks.
Click to expand...

Basically there is nothing wrong with the code, and thus no validation or
error anywhere. It wouldn't be too hard to write a validation rule for it
I guess, but it would generate a number of false positives as well.

To solve it you'll have to overwrite the struct that is in ps[0] with a new
one.

Also because you cannot pass a ref to an indexer or property to a function,
so there basically is no way to get a reference to the original struct.

Jesse

namespace ConsoleApplication1
{
public struct PointD
{
public double X;
public double Y;
public double Z;
public PointD(double x, double y, double z)
{
X = x;
Y = y;
Z = z;
}
public void Set(double x, double y, double z)
{
X = x;
Y = y;
Z = z;
}
}
public class M
{
static public void Main()
{
List<PointD> ps = new List<PointD>();
ps.Add(new PointD(1, 2, 3));
ps.Add(new PointD(4, 5, 6));
ps[0].Set(7, 8, 9);

Console.WriteLine(ps[0].X);
}
}
}
Click to expand...
 
In the following code, the output is 1 instead of 7.  I understand thatthe
generic list<> is generating a temporary copy of PointD, and then the Set()
call works on the temporary and so the result is lost.  It's just subtle
enough though, that unless you are paying really good attention, it's easy to
miss.  FXCop doesn't seem to catch it, not does resharper.  Aside from
changing the struct to a class, other ideas?
Click to expand...

You should never, ever define a struct that is mutable, except for
interop purposes. That's all there is to it. It's probably one of the
more well-known points from the Framework Design Guidelines, and in
general appears in every C#/.NET FAQ that I've seen. The framework
itself breaks it sometimes (e.g. System.Drawing.

At the same time, I'm surprised that FxCop doesn't have a rule for it
(not for your particular case, but against mutable structs in
general). Are you sure you haven't just disabled that?
 
Back
Top