HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Car presenter tests

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
presentercartests

Problem

I caught a whiff of a code-smell emanating from one of my tests, in a scenario akin to the following:

[TestFixture]
public void CarPresenterTests{

    [Test]
    public void Throws_If_Cars_Wheels_Collection_Is_Null(){
         IEnumerable wheels = null;
         var car = new Car(wheels);
         Assert.That(
         ()=>new CarPresenter(car),
         Throws.InstanceOf()
         .With.Message.EqualTo("Can't create if cars wheels is null"));
    }
}

public class CarPresenter{
    public CarPresenter(Car car)
    {
        if(car.Wheels == null)
            throw new ArgumentException("Can't create if cars wheels is null");
        _car = car;
        foreach(var wheel in _car.Wheels)
        {
            wheel.Rolling += WheelRollingHandler;
        }
    }
}


I was struggling to describe what the problem is except that it seems wrong that a CarPresenter should attempt to dictate to a Car whether or not its Wheels are initialised correctly.

I wondered what pointers people here might give me?

Solution

Is a car really a car without wheels?

If not, the check should be done at Car construction time and CarPresenter should not have to check for null, it should assume that a good working car is passed to it. (Correction, CarPresenter should check at construction time that Car is not null.)

Assuming of course that Car is a class that you control as well therefore you can change it.

And while we are talking about smells... You do not have much encapsulation. Wheels on Car should probably be private with public accessor methods if needed.

Answer to comment:


So would you just handle the event
Wheels.Rolling event and accept that
if Wheels is null an exception will be
thrown

No, if you check at construction time and make sure you have working wheels, then you do not need to worry about it later. It is a lot better to bluntly (runtime exception) point out as early as possible if someone made a mistake (passing null for wheels) than waiting for these wheels to blow up who knows where in the code.

Context

StackExchange Code Review Q#3054, answer score: 10

Revisions (0)

No revisions yet.