Are void return methods that change the state of their argument an anti-pattern?

Question

Are methods that return void but change the state of their arguments (ie. provide a hidden or implicit return value) generally a bad practice?

I find them difficult to mock, which suggests they are possibly a sign of a bad design.

What patterns are there for avoiding them?

A highly contrived example:

public interface IMapper
{
    void Map(SourceObject source, TargetObject target);
}

public class ClassUnderTest
{
    private IMapper _mapper;

    public ClassUnderTest(IMapper mapper)
    {
        _mapper = mapper;
    }

    public int SomeOperation()
    {
        var source = new SourceObject();
        var target = new TargetObject();

        _mapper.Map(source, target);

        return target.SomeMappedValue;
    }
}

Answer 1

Yes to some extend.

What you describe is a typical side effect. Side effects make programs hard to understand, because the information you need to understand isn't contained in the call stack. You need additional information, i.e. what methods got called before (and in what) order.

The solution is to program without side effects. This means you don't change variables, fields or anything. Instead you would return a new version of what you normally would change.

This is a basic principle of functional programming.

Of course this way of programming has it's own challenges. Just consider I/O.

Answer 2

Your code whould be a lot easier to test if you do this:

public interface IMapper
{
    TargetObject Map(SourceObject source);
}

public class ClassUnderTest
{
    private IMapper _mapper;

    public ClassUnderTest(IMapper mapper)
    {
        _mapper = mapper;
    }

    public int SomeOperation(SourceObject source )
    {
        var target =  _mapper.Map(source, target);
        return target.SomeMappedValue;
    }
}

You can now test you Map opperation and SomeOperation seperatly. The problem is that you idd change the state of an object which makes it hard to provide a stub for testing. When returning the new object you are able to return a test stub of the target and test your caller method.