-1

I have a list<Izdelki> Kosarica in which I'm adding already created objects from list<Izdelki> lista. But everytime I try to do that, new object nov overwrittes old one, when added to list. Both lists are global, and i create a new object of Izdelek, everytime function is called.

public int izbrani_index;
public List<Izdelek> lista = new List<Izdelek>();
public List<Izdelek> Kosarica = new List<Izdelek>();

protected void GridView1_RowCommand(object sender, GridViewCommandEventArgs e)
{
    int index = Convert.ToInt32(e.CommandArgument);
    izbrani_index = index;

    Izdelek tmp = new Izdelek();
    tmp = lista.ElementAt(index);

    Izdelek nov = new Izdelek();
    nov = tmp; 

    Kosarica.Add(nov); //Object here always rewrites old one
    Session["ses_kosarica"] = Kosarica;

    GridView2.DataSource = null;
    GridView2.DataSource = (List<Izdelek>)Session["ses_kosarica"];
    GridView2.DataBind();
}
johnny 5
  • 19,893
  • 50
  • 121
  • 195
tomazj
  • 303
  • 3
  • 13
  • If it's a list or array, why are you using `ElementAt` ? Besides, you *are* storing the object you returned as `tmp` in the `nov` variable. It's the same object – Panagiotis Kanavos Mar 30 '18 at 15:19
  • 1
    Possible duplicate of [Why does adding a new value to list<> overwrite previous values in the list<>](https://stackoverflow.com/questions/2156482/why-does-adding-a-new-value-to-list-overwrite-previous-values-in-the-list) – Heretic Monkey Mar 30 '18 at 15:21
  • 1
    You can *remove* the `new Izdelek();` expressions since you *always* replace the initial *value* stored in the `tmp` and `nov` *variables*. Once you write `var tmp = lista[index]; var nov=tmp;` it becomes clear you are working with the same object – Panagiotis Kanavos Mar 30 '18 at 15:21
  • btw why are you first creating `new Izdelek` just to assign something else to it? you can just do it like this: `Izdelek tmp = lista[index]`. the same for `nov` – Furkan Kambay Mar 30 '18 at 15:22

5 Answers5

3

These two lines makes no sense

Izdelek tmp = new Izdelek();
tmp = lista.ElementAt(index);

You are creating an object of type Izdelek but the following line replaces the reference to the new object with a reference to an element extracted from lista. The same happens on these two lines

Izdelek nov = new Izdelek();
nov = tmp; 

Now you create another object of type Izdelek but discard the reference and set the variable nov to the same reference of the variable tmp (Extracted from the lista)

Of course when you try to add that element (nov) to the second list you don't have a reference to a new element, but a reference to the same object extracted from the lista.

If you need a new element (a totally separate instance) with the same values from the one extracted from the first list then you need to implements some kind of copy functionality inside the Izdelek class. You can refer to this question for example

Copy constructor vs Clone

Steve
  • 213,761
  • 22
  • 232
  • 286
1

Your line here

Izdelek nov = new Izdelek();
nov = tmp;  //!!! you assign reference

I suppose, considering observed behavior, that Izdelek is a reference type (class), which would produce a wrong result in the moment you execute the line above, as you assign reference of one to another, just created one, by actually overwriting original one.

Tigran
  • 61,654
  • 8
  • 86
  • 123
1

As others already pointed out, you are copying the pointer to the old Izdelek here

nov = tmp;

Thus, any changes you made on nov would be mirrored on tmp as well.

You can create a new Izdelek from the old one in this way:

var nov = new Izdelek
{
   Property = tmp.Property //if property is value type
   Property = new TypeOfProperty //if property is reference type
   {
   ///
   }
}

However, you should the same kind of copying (with new keyword and copying the value type properties) for every reference type property of Izdelek. You can read here more about value and reference type in C#.

http://www.tutorialsteacher.com/csharp/csharp-value-type-and-reference-type

M Bakardzhiev
  • 632
  • 5
  • 13
1

In the code below tmp, and nov are both pointing at the same object in memory--they are pointing at the object returned from lista.ElementAt(index):

Izdelek tmp = new Izdelek();
tmp = lista.ElementAt(index);

Izdelek nov = new Izdelek();
nov = tmp; 

Therefore, the comment in your code below is not a surprise. You are adding the exact same object to Kosarica:

Kosarica.Add(nov); //Object here always rewrites old one
CodingYoshi
  • 25,467
  • 4
  • 62
  • 64
-1

You have to make these lists static the issue is, whenever the event is fired it is recalling the constructor and re initializing the list so making it static will resolve the issue

public int izbrani_index;
    public static List<Izdelek> lista = new List<Izdelek>();
    public static List<Izdelek> Kosarica = new List<Izdelek>();
  • There are no constructors in the event handler. Making a field static won't preserve its value in a *web* application. The application pool can be recycled or the request can go to a different server – Panagiotis Kanavos Mar 30 '18 at 15:22
  • There is always a constructor "The Default Constructor" as we called – Irshad Ahmed Akhonzada Mar 30 '18 at 15:24
  • And it isn't being called in the event constructor **at all**. I suggest you read about the ASP.NET lifecycle. Yes, there is a constructor called *when the webform object is created*, right after the constructor returns. *Nothing* affects it inside the event handler. It *can* retain values, if those are reloaded from viewstate in the Page_Load event. In fact, that's how the old WebForms applications work. – Panagiotis Kanavos Mar 30 '18 at 15:25
  • Finally check the question's code. It reads/writes the list contents from the *Session*. Even if you used static fields a) the values wouldn't be retained and b) they would be overwritten anyway – Panagiotis Kanavos Mar 30 '18 at 15:29
  • static will preserve the list have a look at this demo please https://github.com/irshadahmedevs/eVentureSolutionCodeBank-ASP.Net-/tree/master/CustomValidationDemo – Irshad Ahmed Akhonzada Mar 30 '18 at 15:31
  • I already explained the two reasons why it won't. Assume you are talking to a web developer. There's a *reason* you don't see static fields in web applications. *Especially* in the microservices and serverless era – Panagiotis Kanavos Mar 30 '18 at 15:33
  • Making it `static` will totally change the application and cause many issues because this is an Asp.net application. – CodingYoshi Mar 30 '18 at 15:33
  • I think we are talking in different context – Irshad Ahmed Akhonzada Mar 30 '18 at 15:40
  • If your web site runs for more than 1 day, you'll discover we are not. BTW if you experience constantly increasing memory usage, it's that static list. There's a reason you don't see *any* web site sample using static fields, and why in-memory repositories are *only* used for testing/demos, *always* through dependency injection so they can be removed when not needed – Panagiotis Kanavos Mar 30 '18 at 15:49