Stack overwrites previos push C#

Question

I'm having an issue using a Stack<> object. The push() method is overwriting all my previous values stacked in my Stack object. this is my sample code:

Vehicle veh = new Vehicle();
Stack<Vehicle> StackVeh = new Stack<Vehicle>();

StackVeh.Clear();

veh.Class = "A";
veh.Speed = 280;
veh.Active = true;
StackVeh.Push(veh);

veh.Class = "C";
veh.Speed = 200;
veh.Active = false;
StackVeh.Push(veh);

veh.Class = "B";
veh.Speed = 160;
veh.Active = true;
StackVeh.Push(veh);

veh.Class = "AAA";
veh.Speed = 320;
veh.Active = false;
StackVeh.Push(veh);

foreach (Vehicle v in StackVeh)
{
   Console.WriteLine("\n");
   Console.WriteLine(v.Class);
   Console.WriteLine(v.Speed);
   Console.WriteLine(v.Active);
}

The result I'm having in console is this:

> AAA
> 320
> False
> 
> AAA
> 320
> False
>
> AAA
> 320
> False
>
> AAA
> 320
> False

What am I doing wrong here????

You need to be creating NEW vehicle objects each time before modifying the class, speed, etc. The way it stands you are just adding 4 copies of the same object to the stack. — Matt Runion, Mar 19 '15 at 20:03
In STL C++ this would work, but C# only works with references and you are pushing the same one each time. — Philip Stuyck, Mar 19 '15 at 20:06
Study up on [passing by reference vs passing by value](http://stackoverflow.com/a/430958/2589202). — crthompson, Mar 19 '15 at 20:07
Thank you so much, I see that I'm pushing references to my Stack, and no the values I want. Thanks four your time and knowledge: @PhilipStuyck, and everyone who answered (Sorry if my English is not good) — Jorge Escamilla, Mar 19 '15 at 20:17

score 3 · Answer 1 · answered Mar 19 '15 at 20:06

The problem is never newing your instance of Vehicle. What is happening is that the one instance of vehicle is being pushed onto the stack and then modified, and then added again. The end result is 4 references to the same object pushed on to the stack which in the final form is

veh.Class = "AAA";
veh.Speed = 320;
veh.Active = false;

Your code should be the following

Vehicle veh = new Vehicle();
Stack<Vehicle> StackVeh = new Stack<Vehicle>();

StackVeh.Clear();

veh.Class = "A";
veh.Speed = 280;
veh.Active = true;
StackVeh.Push(veh);

veh = new Vehicle();

veh.Class = "C";
veh.Speed = 200;
veh.Active = false;
StackVeh.Push(veh);

veh = new Vehicle();

veh.Class = "B";
veh.Speed = 160;
veh.Active = true;
StackVeh.Push(veh);

veh = new Vehicle();

veh.Class = "AAA";
veh.Speed = 320;
veh.Active = false;
StackVeh.Push(veh);

foreach (Vehicle v in StackVeh)
{
   Console.WriteLine("\n");
   Console.WriteLine(v.Class);
   Console.WriteLine(v.Speed);
   Console.WriteLine(v.Active);
}

This will make sure you are using a new instance of Vehicle and not assigning new values to a single instance.

Rufus L · Answer 2 · 2015-03-19T20:32:22.233

You need to set veh to a new Vehicle, since you are passing an object reference. The push is not overwriting anything, it's the fact that you keep updating the same object instead of creating new ones...

Your creation code can be simplified a little, too:

var vehicles = new Stack<Vehicle>();

var veh = new Vehicle {Class = "A", Speed = 280, Active = true};
vehicles.Push(veh);

veh = new Vehicle {Class = "C", Speed = 200, Active = false};
vehicles.Push(veh);

veh = new Vehicle {Class = "B", Speed = 160, Active = true};
vehicles.Push(veh);

veh = new Vehicle {Class = "AAA", Speed = 320, Active = false};
vehicles.Push(veh);

foreach (var vehicle in vehicles)
{
    Console.WriteLine();
    Console.WriteLine(vehicle.Class);
    Console.WriteLine(vehicle.Speed);
    Console.WriteLine(vehicle.Active);
}

In fact, you don't really even need the veh variable at all, you can just push new Vehicles directly, reducing your code from the original 20 lines to 5:

var vehicles = new Stack<Vehicle>();
vehicles.Push(new Vehicle {Class = "A", Speed = 280, Active = true});
vehicles.Push(new Vehicle {Class = "C", Speed = 200, Active = false});
vehicles.Push(new Vehicle {Class = "B", Speed = 160, Active = true});
vehicles.Push(new Vehicle {Class = "AAA", Speed = 320, Active = false});

You also might consider overriding the ToString() method on your Vehicle class:

class Vehicle
{
    public string Class { get; set; }
    public int Speed { get; set; }
    public bool Active { get; set; }

    public override string ToString()
    {
        return string.Format("Class: {0}, Speed: {1}, Active: {2}", 
            Class, Speed, Active);
    }
}

So that you can just do something like:

for (int i = 0; i < vehicles.Count; i++)
{
    Console.WriteLine("Vehicle #{0} properties: {1}", i + 1, vehicles.ElementAt(i));
}

Stack overwrites previos push C#

2 Answers2