jeudi 21 janvier 2016

Using mutable objects as constructor arguments

What is the best practice to pass objects as constructor arguments? Passing mutable objects can lead to unexpected results.

A simple example. We expect 200, but get 10000 calling the TestMethod():

public class Test
{
    public int TestMethod()
    {
        var variable1 = new SomeMeasurements
        {
            Width = 10,
            Height = 20
        };
        var obj1 = new MyRectangle(variable1);

        // <... more code...>

        variable1.Height = 1000; // a local variable was reused here, and it's field was changed

        // <... more code...>

        return obj1.GetArea();
    }
}

public class SomeMeasurements
{
    public int Width { get; set; }
    public int Height { get; set; }
}


public class MyRectangle
{
    SomeMeasurements _arg;

    public MyRectangle(SomeMeasurements arg)
    {
        _arg = arg;
    }

    public int GetArea()
    {
        return _arg.Width * _arg.Height;
    }
}

In this case the error is obvious, but with more complex classes debugging can be tedious. Several things how to fix this have crossed my mind:

option 1. Fix TestMethod() - it mustn't change variable1 after creating MyRectangle.

option 2. Fix class SomeMeasurements - turn it into a struct:

public struct SomeMeasurements
{
    public int Width { get; set; }
    public int Height { get; set; }
}

option 3. Fix class SomeMeasurements - make it immutable:

public class SomeMeasurements
{
    public SomeMeasurements(int width, int height)
    {
        Width = width;
        Height = height;
    }

    public int Width { get; }
    public int Height { get; }
}

option 4. Fix class MyRectangle body - it mustn't use mutable objects:

public class MyRectangle
{
    int _height;
    int _width;

    public MyRectangle(SomeMeasurements arg)
    {
        _height = arg.Height;
        _width = arg.Width;
    }

    public int GetArea()
    {
        return _width * _height;
    }
}

option 5. Make SomeMeasurements ICloneable and use it's Clone() in MyRectangle constructor.

Any of these options have it's flaws - it might be hard to avoid reusing variable1, MyRectangle can be more complex to turn it into a struct, MyRectangle can be external and you might not change it at all, etc. What is the most correct way to fix this?

Aucun commentaire:

Enregistrer un commentaire