Why is this method Impure?

Question

I read this answer: https://stackoverflow.com/a/9928643/16241

But I obviously don't understand it because I can't figure out why my method is impure. (The method in question is ToExactLocation()).

public struct ScreenLocation
{
    public ScreenLocation(int x, int y):this()
    {
        X = x;
        Y = y;
    }

    public int X { get; set; }
    public int Y { get; set; }

    public ExactLocation ToExactLocation()
    {
        return new ExactLocation {X = this.X, Y = this.Y};
    }

    // Other stuff
}

Incase you need it here is the exact location struct:

public struct ExactLocation
{
    public double X { get; set; }
    public double Y { get; set; }

    // Various Operator Overloads, but no constructor
}

And this is how I call it:

someScreenLocation = MethodThatGivesAScreenLocation();
if (DestinationLocation == someScreenLocation.ToExactLocation())
{
     // Do stuff
}

When I do that, ReSharper flags it with "Impure Method is called for readonly field of value type."

Why is it saying that? And what can I do to make it go away?

Answer 1

It's not pure because it does not return a value dependent only on its input. When the value of X or Y changes so does the return value of ToExactLocation, i.e., its output depends on internal, mutable state.

Additionally, the setters for X or Y in ExactLocation may mutate the input. The getters of ScreenLocation may as well.

someScreenLocation is a readonly field and is a value type. You are calling ToExactLocation on a value, i.e., a readonly field. When you access a reaodnly value type a copy is created as to avoid mutating the value itself. However, your call may mutate that value, which, in many cases, is not what you want as you will be mutating a copy. This is why you get a warning.

In this case, you can ignore it, but I would avoid mutable value types in general.

EDIT:

Let me attempt to simplify...

struct Point
{
    int X;
    int Y;
    bool Mutate() { X++; Y++; }
}

class Foo
{
    public readonly Point P;
    Foo() 
    { 
        P = new Point();
        P.Mutate();  // impure function on readonly value type
    }
}

When Mutate() is called, a copy of P is created and passed along with the method. Any mutation of P's internal state will be irrelevant as it mutates a copy.

Answer 2

One of the conditions of a Pure Method is that its output (return value) is wholly dependent on its input (arguments).

Your .ToExactLocation() method is not pure, because its output depends both on the input arguments and also on the current value of a mutable struct.

Resharper doesn't like this, because mutable structs are bad (don't use them). I expect the error would go away if you either changed your code to use a class instead of a struct or redesigned the struct so the the .X and .Y members could only be set by the constructor.

Answer 3

Reading the answer, I found out that pure functions are necessarily like functions in mathematics. f(x) = x^2 + 2x + 10 will always return 10 if x is 0.

So ToExactLocation() must return the same values each time it is called, regardless changes to object since initial creation, for it to be called "pure".

Answer 4

There are 2 meaning of "pure function": one theoretical (no side effects/no dependency on mutable state) and another is what ReSharper thinks about functions.

From theoretical point of view your function is not pure because it depends on mutable state. Sample:

var someScreenLocation = new ScreenLocation(1,1);

var locationOne = someScreenLocation.ToExactLocation();
var locationTwo = someScreenLocation.ToExactLocation();
someScreenLocation.X = 3;
var locationThree = someScreenLocation.ToExactLocation();

For method to be pure it can change its result only based on input (not at all as in this case since there is no arguments). But you can clearly observe that locationOne and locationTwo are the same (good sign so far), but unfortunately locationThree is different even if the input (arguments to the function) still the same.

You can make it theoretically pure by making X and Y readonly (and adding constructor).

Even after the change ReSharper will still think it is not pure - to convince it you can use Pure attribute to mark it as pure.

Note that ReSharper marks usage of "impure" functions even in constructor of the class with readonly field. Sample below shows ReSharper warnings:

struct Point
{
    public int X;
    public int Y;
    public Point(int x, int y){X = x;Y = y;}

    public void Mutate(){X++;}
    public Point TheoreticallyPure(){return new Point(1, 1);}
    [Pure] public Point MarkedPure(){ return new Point(1, 1);}
}

class WithReadonlyField
{
    public readonly Point P;
    public WithReadonlyField()
    {
        P = new Point();
        P.TheoreticallyPure();  // impure function on readonly value type
        P.MarkedPure(); // return value of pure not used
        P.Mutate();   // impure function on readonly value type - modifies P.
        P = new Point().MarkedPure(); // ok to modify P multiple times.
    }
    public void NormalMethod()
    {
        P.Mutate();   // impure function on readonly value type, no changes to P
    }
}

C# allows modification of readonly fields up to the end of constructor, but ReSharper marks usages of all "impure" functions there too (Note that Mutate function in constructor actually changes value of readonly field P, unlike in NormalMethod where it has no effect).

"readonly... assignments to the fields introduced by the declaration can only occur as part of the declaration or in a constructor in the same class"

Most likely this behavior of ReSharper is for consistency and to avoid cases where moving perfectly valid code changes behavior completely.

Answer 5

It would be better to model this as a static method (on either class) and would get rid of the impure warning. Explanation omitted, as the other answers covers the why already.

Example:

public static ExactLocation ToExactLocation(ScreenLocation loc)
{
    return new ExactLocation {X = loc.X, Y = loc.Y};
}

or use an extension method

public static ExactLocation ToExactLocation(this ScreenLocation loc)
{
    return new ExactLocation {X = loc.X, Y = loc.Y};
}

Answer 6

Not really sure about the cause, and I'd put this as a comment if it would format correctly...

Wouldn't you want something like:

var someScreenLocation = MethodThatGivesAScreenLocation();
if (DestinationLocation.X == someScreenLocation.ToExactLocation().X &&
    DestinationLocation.Y == someScreenLocation.ToExactLocation().Y)
{
     // Do stuff
}