Search code examples
c#pass-by-referencereadability

Maximizing code readability in methods with side effects?


I'm working with a method that causes side effects on a passed reference parameter. One of my primary concerns for the code is readability, and this is a situation that I feel is a potential cause for confusion.

Take for example:

class Program
{
    static void Main(string[] args)
    {
        var simplePOCO = new SimplePOCO();

        // Method 1: causes side effects, but potentially unclear
        Method1(simplePOCO);

        // Method 2: assignment makes intentions clearer, but unnecessary
        simplePOCO = Method2(simplePOCO);

        // Method 3: ref/out makes intentions clearer, but (very?) unnecessary
        Method3(ref simplePOCO);

        // Method 4: avoid the problem altogether
        simplePOCO.SimpleProperty = Method4();
    }

    public static void Method1(SimplePOCO simplePOCO)
    {
        simplePOCO.SimpleProperty = 1;
    }

    public static SimplePOCO Method2(SimplePOCO simplePOCO)
    {
        simplePOCO.SimpleProperty = 1;
        return simplePOCO;
    }

    public static SimplePOCO Method3(ref SimplePOCO simplePOCO)
    {
        simplePOCO.SimpleProperty = 1;
        return simplePOCO;
    }

    public static int Method4()
    {
        return 3;
    }
}

class SimplePOCO
{
    public int SimpleProperty { get; set; }
}

I'm leaning toward using method 2, but I realize it's just using a self-assignment. Method 4 also looks good but in this case will take a little refactoring to get there - is it worth it? I'm curious if anyone has any strong feelings one way or another about this. Obviously, proper naming of the method will go a long way. Are there or some school of thought that addresses this concern? Are there any other styles I haven't thought of?


Solution

  • If this is the only side effect I would put the method were it belongs: inside SimplePOCO so that the method and the data it modifies are encapsulated together:

    class SimplePOCO
    {
        public int SimpleProperty { get; set; }
        public void Method5()
        {
           SimpleProperty = 3;
        }
    }
    

    Also the method name should indicate that a change is to be expected as a result of the call, i.e. UpdateSimplePropertyRandomly().