Property set, null in other class. C#

Question

Okay, I have no idea why this happens.

I set a property and just after I set it, it's null in an other class. Stepping though it with the debugger just shows it's set, and that its null in the other class.

This is my code: (stripped away all unnecessary code)

public class SnakeGame : SenseHatSnake, ISnakeGame
{
    private readonly IBody _body;

    public SnakeGame(ISenseHat senseHat, IBody body)
        : base(senseHat)
    {
        _body = body;
    }

    private void UpdateGame(Object state)
    {

        _movement.PreviousPositions.Add(new Position()
        {
            X = _movement.X,
            Y = _movement.Y
        });

        if (_body.DetectCollision(_movement.X, _movement.Y))
        {
            GameOver();
        }
    }
}

It happens at DetectCollision in the body class.

public class Body : IBody
    {    
        private readonly IMovement _movement;

        public Body(IMovement movement)
        {
            _movement = movement;
        }

        public bool DetectCollision(int x, int y)
        {
            for (int i = 1; i < Length + 1; i++)
            {
                if (_movement.PreviousPositions.Count > i)
                {
                    int bX = _movement.PreviousPositions[_movement.PreviousPositions.Count - i - 1].X;
                    int bY = _movement.PreviousPositions[_movement.PreviousPositions.Count - i - 1].Y;

                    if (bX == x && bY == y)
                    {
                        return true;
                    }
                }
            }

            return false;
        }

    }

I have just set the _movement.PreviousPositions and I can see it in debugger, but as soon it's in Body _movement.PreviousPositions is null.

Movement.cs:

public class Movement : IMovement
{
    public List<Position> PreviousPositions { get; set; }      
}

Note:

I am using DI, am I doing something wrong there? (Autofac)

public sealed partial class MainPage : Page
{
    public MainPage()
    {
        this.InitializeComponent();



        var builder = new ContainerBuilder();
        builder.RegisterType<Body>().As<IBody>();
        builder.RegisterType<Display>().As<IDisplay>();
        builder.RegisterType<Draw>().As<IDraw>();
        builder.RegisterType<Food>().As<IFood>();
        builder.RegisterType<Movement>().As<IMovement>();
        builder.RegisterType<SnakeGame>().As<ISnakeGame>();
        builder.RegisterType<ISenseHat>().As<ISenseHat>();
        var container = builder.Build();

        var body = container.Resolve<IBody>();
        var display = container.Resolve<IDisplay>();
        var draw = container.Resolve<IDraw>();
        var food = container.Resolve<IFood>();
        var movement = container.Resolve<IMovement>();

        Task.Run(async () =>
        {
            ISenseHat senseHat = await SenseHatFactory.GetSenseHat().ConfigureAwait(false);
            var snakeGame = new SnakeGame(senseHat, body, display, draw, food, movement);
            snakeGame.Run();
        });


    }


}

`PreviousPositions` in Movement. (will add that class to the question) — Luuk Wuijster, Aug 31 '18 at 01:52
`SnakeGame` doesn't define `_movement` in your question, so it's unclear how this relates to `_body`. — ProgrammingLlama, Aug 31 '18 at 01:53
try putting a breakpoint to the setter of `PreviousPositions`. — Selman Genç, Aug 31 '18 at 01:53
If you're assuming that `Body` will get the same `IMovement` as `SnakeGame`, that's incorrect. Autofac's default registration is `InstancePerDependency`, so `Body` and `SnakeGame` will each get their own unique `Movement` instance. Is that the issue here? — ProgrammingLlama, Aug 31 '18 at 01:57
Hmmmm... Okay, I did not know that. Is there a way they can get the same instance? — Luuk Wuijster, Aug 31 '18 at 01:59

score 2 · Accepted Answer · answered Aug 31 '18 at 02:04

By default, Autofac registers components as InstancePerDependency, so this code will obtain two unique instances:

var a = container.Resolve<IExample>();
var b = container.Resolve<IExample>();

If you want a singleton, you need to register the object as such by using .SingleInstance():

builder.RegisterType<Movement>().As<IMovement>().SingleInstance();

This ought to resolve your issue, although I have noticed that you seem to be using the service locator anti-pattern by manually resolving each service. You can change your code to be like this:

var builder = new ContainerBuilder();
builder.RegisterType<Body>().As<IBody>();
builder.RegisterType<Display>().As<IDisplay>();
builder.RegisterType<Draw>().As<IDraw>();
builder.RegisterType<Food>().As<IFood>();
builder.RegisterType<Movement>().As<IMovement>().SingleInstance();
builder.RegisterType<SnakeGame>().As<ISnakeGame>();
builder.RegisterInstance<ISenseHat>(await SenseHatFactory.GetSenseHat().ConfigureAwait(false));

var container = builder.Build();

var snakeGame = container.Resolve<ISnakeGame>();
snakeGame.Run();

Mehdi Ibrahim · Answer 2 · 2018-08-31T04:18:08.473

-1

RegisterType will resolve to a new instance if a type is requested. If you want to share a single instance across your classes, use RegisterInstance instead for your Movement class e.g.

        var movement = new Movement();
        var builder = new ContainerBuilder();
        builder.RegisterType<Body>().As<IBody>();
        builder.RegisterType<Display>().As<IDisplay>();
        builder.RegisterType<Draw>().As<IDraw>();
        builder.RegisterType<Food>().As<IFood>();
        **builder.RegisterInstance<IMovement>(movement);**
        builder.RegisterType<SnakeGame>().As<ISnakeGame>();
        builder.RegisterType<ISenseHat>().As<ISenseHat>();
        var container = builder.Build();

edited Aug 31 '18 at 04:18

answered Aug 31 '18 at 02:04

Mehdi Ibrahim

2,434
1
13
13

This is a bad solution because that prevents you using dependency injection in the type you're adding via `RegisterInstance`. Registering the type and calling `.SingleInstance()` will allow you to still inject services into the singleton instance. Thanks for the downvote though :) – ProgrammingLlama Aug 31 '18 at 02:05
There's a reason why RegisterInstance exists - specifically to solve these types of problems. Dependency Injection exists to maintain your dependency graph and instance management in one place. Please back up your comment and downvote with a good explanation. – Mehdi Ibrahim Aug 31 '18 at 02:08
I have provided a good explanation. In fact, I've used `RegisterInstance` in my own example - to register an instance provided by an external service. If OP uses any injected services in the `Movement` class, OP can't use `RegisterInstance` to register it. It's as simple as that. `.SingleInstance()` was created to solve this problem. – ProgrammingLlama Aug 31 '18 at 02:08
Do you have any evidence to back up `RegisterInstance` being the superior solution here? :) – ProgrammingLlama Aug 31 '18 at 02:14
So just because I say to use it, it's bad use? And because you're saying to use it, it's justified? How is a Singleton better than a single Instance from the perspective of your explanation. I'm not sure if you have a correct understanding of DI -or- you just enjoy downvoting other people who are out there to help the community :) – Mehdi Ibrahim Aug 31 '18 at 02:14
Mehdi, how do you use injected services inside an instance registered using `RegisterInstance`? – ProgrammingLlama Aug 31 '18 at 02:16
Movement does not have any injected services! – Mehdi Ibrahim Aug 31 '18 at 02:18
"This is my code: (stripped away all unnecessary code)" - it might do. I'm not sure why you (Mehdi) would use `RegisterInstance` over calling `.SingleInstance()` really. Anyway, you can read more about them [here](https://stackoverflow.com/questions/31582000/autofac-registerinstance-vs-singleinstance), [here](https://autofaccn.readthedocs.io/en/latest/register/registration.html#instance-components) and [here](https://autofaccn.readthedocs.io/en/latest/lifetime/instance-scope.html#single-instance). P.S. You ought to downvote answers on their merit, not your feelings. – ProgrammingLlama Aug 31 '18 at 02:19
I did not downvote, but I know you downvoted my answer. You have not given a valid explanation of why RegisterInstance for Movement would not work. Yet you claim this is a "bad solution". I ought to downvote your answer for this, but I won't :) – Mehdi Ibrahim Aug 31 '18 at 02:28
OK I appreciate that you didn't downvote mine. Anyway, my downvote comes down to: 1) this is a QA site, and while it might be an appropriate option for OP's answer (assuming Movement in the Q is the full code), it isn't a universally appropriate solution. 2) RegisterInstance requires you to instantiate the object at the point where you create the container, which if the object is seldom used then it could add to startup time unnecessarily. 3) Unless you're registering an external service, `SingleInstance()` is a better general solution (especially since you can inject services into it). – ProgrammingLlama Aug 31 '18 at 02:31
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/179144/discussion-between-john-and-mehdi-ibrahim). – ProgrammingLlama Aug 31 '18 at 02:39
@John, answers are given within the context of questions posted. Instantiating a new instance of Movement will absolutely NOT incur additional start up time and most certainly would NOT be a more expensive operation than what you have proposed. You're also wrong about one being a better solution than the other - both will result in a singleton being used. You're misleading readers. – Mehdi Ibrahim Aug 31 '18 at 04:21

Property set, null in other class. C#

2 Answers2