is there a better way to test this method?

Question

I would like to run some unit tests on the method below I am passing a mocked interface(vehicleObject) into ProcessVehicles but as soon as it is passed it gets reassigned by DetermineVehicleType so my mocked object is of no use. My first idea would be to create a boolean to determine if DetermineVehicleType should be run and add it as a param, but that sounds so very messy. Is there a better way to get around this ?

Method with Mock object being injected:

public void ProcessVehicles(ICarObject CarObject)
{
    IObject vehicleObject = DetermineVehicleType(carObject);
    vehicleObject.ProcessVehicle(carObject);
}

Original Code:

public void ProcessVehicles()
{
    IObject vehicleObject = DetermineVehicleType(carObject);
    vehicleObject.ProcessVehicle(carObject);
}

Note: I can't check if vehicleObject is null before calling DetermineVehicleType because it might not be null when the class is actually used. In the long run maybe total refactor is the answer, at this point that is not the answer I am looking for maybe there is not another option.

the method DetermineVehicleType is private

Note: I know there are code smells this is legacy code that currently works. I want to get tests around it not change it so it looks pretty and then breaks in production. Total refactor might be the only option I just want to make sure there is not another solution with the mock tools.

Answer 1

What access modifier does DetermineVehicleType have? You could stub out that method so that it returns your mocked interface (Roy Osherove calls this the abstract test driver pattern, I believe). Otherwise, this looks like a prime candidate for refactoring :)

To refactor your code you would do something like this

First, change your method signature

protected virtual IObject DetermineVehicleType(CarObject obj)
{
    //Do whatever you normally do
}

Then, in your test, you can create a stub out of the above class, and have it return your stubbed IObject no matter the CarObject passed in. You can either manually create a stub class by inheriting from the class, or you could use something like MOQ to accomplish this. Let me know if you need me to elaborate on this a little more.

Another note, however:

A better way to refactor this would be to simply pass in the IObject to the ProcessVehicles, as it seems from this example that you have a SRP violation here, where the ProcessVehicles method is doing more than processing them. But, maybe that is just from this simplified example

FULL Implementation Update

    [Test]
    public void TestMethod()
    {
        var testerStub = new TesterStub();
        testerStub.ProcessVehicles();
        //Assert something here
    }

    public class TesterStub : Tester
    {
        public override IObject DetermineVehicleType(CarObject obj)
        {
            var mockObject = new Mock<IObject>();
            mockObject.Setup(x => x.SomeMethod).Returns(Something);
            return mockObject.Object;
        }
    }

    public class Tester
    {
        protected virtual IObject DetermineVehicleType(CarObject obj)
        {
            return new ObjectTester();
        }

        public void ProcessVehicles()
        {
            var carType = DetermineVehicleType(new CarObject());

        }
    }

    public class ObjectTester : IObject
    {
    }

    public interface IObject
    {
    }

    public class CarObject
    {
    }

Answer 2

If we are being very literal, I believe your code demonstrates a smell.

Consider this: the method ProcessVehicles calls a method on the instance called DetermineVehicleType. What does your class do? Does it Process Vehicles, or does it Determine Vehicle Type? This to me indicates violation of SRP, if you take it literally. Your class is trying to do more than one job.

Arguably this implies that SRP disapproves of private helper methods. Some commentators do indeed hold this to be the case. I'm not sure I do; as always, common sense is key.

If I were to refactor this code, I would give the class something like an IVehicleCategoryHelper which exposes DetermineVehicleType. Perhaps this would be passed in through its constructor, or if we are implementing full-on Fat Dependency Injection, an IFactory so that the instance can retrieve an IVehicleCategoryHelper when it needs one, dependent on context.

Take everything I've said with a pinch of salt. I don't necessarily believe that this is the right approach - it will ultimately be up to you to decide.