C# why is my operator overload mutating the object rather than returning a new one?

Question

I'm working on a matrix/vector class(es) and part of it has operator overloading. One of the odd things is, that instead of returning a new object (in this case vector) it's mutating the vector, AND returning a new one. How do I cleanly return a new instance of my Vector?

here are the constructors:

    private List<double> v = new List<double>();
    private int dims;


    public Vector(double[] a)
    {
        foreach (double d in a)
        {
            v.Add(d);
            dims++;
        }
    }

    public Vector(int dims)
    {
        var a = new double[dims];
        v = a.ToList();
        this.dims = dims;
    }

    private Vector(List<double> a)
    {
        v = a;
        dims = a.Count;
    }

and the operator overloads (just posting addition because all of them have the problem, and have similar construction, so should have identical solutions)

    public static Vector operator + (Vector a, Vector b)
    {
        Vector c = new Vector();
        c = a;
        for (int dim = 0; dim < c.dims; dim++)
        {
            c[dim] += b[dim];
        }
        return c;
    }

EDIT: So, I've changed the class to a struct, and it seems like I still have the issue. Maybe it''s because the variable v is a list, and therefore it's a class, it's still passing in the reference to the list? Maybe I have to use an array instead of a list?

Answer 1

The line:

c = a;

is the problem.

Both c and a are then pointing to the same object.

Possible solutions:

1) Change Vector from a class to a struct.

2) Use one of your constructors to create a new Vector object:

public static Vector operator + (Vector a, Vector b)

{
    Vector c = new Vector(a.v);

    for (int dim = 0; dim < c.dims; dim++)
    {
        c[dim] += b[dim];
    }
    return c;
}

Answer 2

The problem is your c = a which "copy" the address instead of values. Your + operator should look like this:

public static Vector operator + (Vector a, Vector b)
{
    if (a.dims != b.dims)
        throw new WhatEverException();

    Vector c = new Vector(a.dims);
    // c = a
    for (int dim = 0; dim < c.dims; dim++)
    {
        // c[dim] += b[dim];
        c[dim] = a[dim] + b[dim];
    }
    return c;
}

Little plus: you can change your dims member to a readonly property:

private int dims { get { return v.Count; } }

---

How to test:

    public class Vector
    {
        public char Id { get; set; } // just for debug
        public double this[int i] { get { return Values[i]; } set { Values[i] = value; } }
        private List<double> Values { get; set; }
        public int Dim { get { return Values.Count; } }
        public Vector(double[] values) { Values = values.ToList(); }
        public Vector(int dims) { Values = new double[dims].ToList(); }

        // note this constructor, the one you posted actually copy the whole list object
        public Vector(List<double> values) { Values = new List<double>(values); }

        public static Vector operator +(Vector a, Vector b)
        {
            if (a.Dim != b.Dim)
                throw new Exception();
            Vector c = new Vector(a.Dim) { Id = 'c' };
            for (int i = 0 ; i < c.Dim ; i++)
                c[i] = a[i] + b[i];
            return c;
        }
    }

    static void Main(string[] args)
    {
        Vector a = new Vector(new double[] { 1.42857, 1.42857, 1.42857 }) { Id = 'a' };
        Vector b = new Vector(new double[] { 1.42857, 2.85714, 4.28571 }) { Id = 'b' };
        Vector c = a + b;
        Console.WriteLine(string.Format(">{0} = {1} + {2}", c.Id, a.Id, b.Id));
        Console.WriteLine(string.Format(">{1} + {2} = {0}", c[0], a[0], b[0]));
        Console.WriteLine(string.Format(">{1} + {2} = {0}", c[1], a[1], b[1]));
        Console.WriteLine(string.Format(">{1} + {2} = {0}", c[2], a[2], b[2]));
        Console.ReadKey();
    }

Result:

>c = a + b
>1.42857 + 1.42857 = 2.85714
>1.42857 + 2.85714 = 4.28571
>1.42857 + 4.28571 = 5.71428

Answer 3

You should either copy all vector a values to Vector c, or change Vector to struct instead of class.

Doing a = c, makes c reference a, therefore all modifications to c is also applied to a. It happens because Vector is a class and it is passed around as a reference rather than value.

One way to work around this is to loop through all a values and add them to c. Or, better, you have constructor, that does that for you, so your end operator override should look like

public static Vector operator + (Vector a, Vector b)
{
    // Will work, because you are still inside Vector class and you can access all private members.
    Vector c = new Vector(a.v);  

    for (int dim = 0; dim < c.dims; dim++)
        c[dim] += b[dim];

    return c;
}

Answer 4

You can use Zip to add elements of two lists. Also, you could use AddRange instead of loops.

class Vector
{
    private List<double> v = new List<double>();
    private int dims;

    public Vector(double[] a)
    {
        v.AddRange(a);
    }

    public Vector(int dims)
    {
        var a = new double[dims];
        v = a.ToList();
        this.dims = dims;
    }

    private Vector(List<double> a)
    {
        v = new List<double>(a);
        dims = a.Count;
    }

    public static Vector operator +(Vector a, Vector b)
    {
        var ls = a.v.Zip(b.v, (x, y) => x + y).ToList();
        return new Vector(ls);
    }
}

Answer 5

allright, I've finaly worked something out. it's a tad messy but it works. and I have no idea why I have to jump through so many hoops in order to get this thing working, but here's my solution:

    public static Vec operator + (Vec a, Vec b)
    {
        Vec c = new Vec();
        c.v = new List<double>();
        for (int dim = 0; dim < a.Dims; dim++)
        {
            c.v.Add(a[dim] + b[dim]);
        }
        return c;
    }

I'm unsure if there is a more elegant way to do this, but hey, it works. I've also made it a struct rather than a class.