Why does adding values to a collection overwrites previous items and how to simplify?

Question

Based on an abstract class the programs adds values to a collection. Problem 1: When displaying the added values they all are over written with the latest added value. As a side problem, adding the values seems to tedious, there must be better way to achieve this.

Browsing through other answers, there are similar issues using a static class, however that is not the case here. I tried with removing the "abstract" which makes no difference in output.

    using System;
    using System.Collections.Generic;
    using System.Collections.ObjectModel;
    using System.Linq;
    using System.Text;
    using System.Threading.Tasks;

namespace QuoteCoScMe
{
    class Program
    {
        public abstract class Koers
        {
            public string fonds { get; set; }
            public DateTime datum { get; set; }
            public Double koers { get; set; }
        }
        public class Historical : Koers
        {

        }

        private static void Display(Collection<Historical> cs)
        {
            Console.WriteLine();
            foreach (Historical item in cs)
            {
                Console.WriteLine("{0} {1} {2} ", item.fonds, item.datum.ToString(), item.koers);
            }
        }

        static void Main(string[] args)
        {
            Historical xkoers = new Historical() ;
            Collection<Historical> Historicals = new Collection<Historical>();
            xkoers.fonds = "AL1";
            xkoers.datum = DateTime.Parse("2018-05-08");
            xkoers.koers = 310.1;
            Historicals.Add(xkoers);
            xkoers.fonds = "AL2";
            xkoers.datum = DateTime.Parse("2018-06-08");
            xkoers.koers = 320.1;
            Historicals.Add(xkoers);
            xkoers.fonds = "Some other 3";
            xkoers.datum = DateTime.Parse("2019-06-08");
            xkoers.koers = 20.1;
            Historicals.Add(xkoers);
            Display(Historicals);
            /* Question 2: this is a tedious way of adding, i would want to use xkoers.add("AL2", DateTime.Parse("2018-05-08"), 320); */
            /* Question 1: when displaying the historicals for some reason the whole list contains only the latest added item in the list.
               In de VS debugger is shows that all list items have the same values. 

            Output:
                Some other 3 8/06/2019 0:00:00 20,1
                Some other 3 8/06/2019 0:00:00 20,1
                Some other 3 8/06/2019 0:00:00 20,1
                Press any key to continue . . .
             */

        }
    }

}

score 1 · Answer 1 · edited Nov 19 '21 at 08:27

1

You have one bucket:

Historical xkoers = new Historical() ;

and you fill it up three times.

You need to new up the variable each time you add it:

xkoers = new Historical() ;
xkoers.fonds = "AL1";
xkoers.datum = DateTime.Parse("2018-05-08");
xkoers.koers = 310.1;
Historicals.Add(xkoers);

xkoers = new Historical() ;
xkoers.fonds = "AL2;
xkoers.datum = DateTime.Parse("2018-05-08");
xkoers.koers = 310.1;
Historicals.Add(xkoers);

// etc

As for your second problem, you could use a constructor.

edited Nov 19 '21 at 08:27

halfer

19,824
17
99
186

answered Mar 17 '19 at 08:31

TheGeneral

79,002
9
103
141

The collection is basically a list of pointers and what i did is store the same pointer over and over. – Maarten Daniels Mar 19 '19 at 10:06

score 1 · Answer 2 · answered Mar 17 '19 at 08:34

Regarding adding items, you can use Object initializers and collection initializers

var Historicals = new Collection<Historical>()
{
   new Historical() { fonds = "AL1", datum = DateTime.Parse("2018-05-08"), koers = 310.1),
   new Historical() { fonds = "AL2", datum = DateTime.Parse("2018-06-08"), koers = 310.1)
};

score 0 · Answer 3 · answered Mar 17 '19 at 08:35

0

The reason why it is being duplicated is because you are using the same instance ( reference to object in memory)

so every time you change xkoers you end up changing the one in reference which all added items in historicals are pointing to...

Historical xkoers = new Historical() ;
Collection<Historical> Historicals = new Collection<Historical>();
xkoers.fonds = "AL1";
xkoers.datum = DateTime.Parse("2018-05-08");
xkoers.koers = 310.1;

can be done like this

xkoers = new Historical() { fonds = "AL1", datum = DateTime.Parse("2018-05-08"), koers = 310.1)

which will change the point of refence to a new place in memory

then add that to your list and it should not repeat.

answered Mar 17 '19 at 08:35

Neil

641
1
7
21

1

go check whether a Collection is really what you need in this case. https://stackoverflow.com/questions/271710/collectiont-versus-listt-what-should-you-use-on-your-interfaces – Neil Mar 17 '19 at 08:40
one more side note. you are afrikaans (but this applies to any language). You have to consider if your code will ever be taken over by someone and will that person be able to understand afrikaans. naming convention is important. – Neil Mar 17 '19 at 08:42
I did the research, collections is better replaced with a list – Maarten Daniels Mar 19 '19 at 10:03

score 0 · Answer 4 · answered Mar 24 '19 at 18:18

With information received from all 3 contributers, some reading on C and c# on variables en datatypes, i compiled the following that exactly does what i want.

class Program
{
    public abstract class Koers
    {
        public string fonds { get; set; }
        public DateTime datum { get; set; }
        public Double koers { get; set; }
    }
    public class Historical : Koers
    {

    }

    private static void Display(List<Historical> cs)
    {
        Console.WriteLine();
        foreach (Historical item in cs)
        {
            Console.WriteLine("{0} {1} {2} ", item.fonds, item.datum.ToString(), item.koers);
        }
    }

    static void Main(string[] args)
    {
        Historical xkoers = new Historical();
        List<Historical> Historicals = new List<Historical>();

        Historicals.Add ( new Historical() { fonds = "EL1", datum = DateTime.Parse("2018-05-08"), koers = 310.1 } ) ;
        Historicals.Add ( new Historical() { fonds = "EL2", datum = DateTime.Parse("2018-06-08"), koers = 311.1 } ) ;
        xkoers.fonds = "AL3";
        xkoers.datum = DateTime.Parse("2018-05-08");
        xkoers.koers = 310.1;
        Historicals.Add(new Historical() { fonds=xkoers.fonds, datum=xkoers.datum, koers = xkoers.koers });
        xkoers.fonds = "AL4";
        xkoers.datum = DateTime.Parse("2018-06-08");
        xkoers.koers = 320.1;
        Historicals.Add(new Historical() { fonds = xkoers.fonds, datum = xkoers.datum, koers = xkoers.koers });
        xkoers.fonds = "Some other 5";
        xkoers.datum = DateTime.Parse("2019-06-08");
        xkoers.koers = 20.1;
        Historicals.Add(new Historical() { fonds = xkoers.fonds, datum = xkoers.datum, koers = xkoers.koers });

        Display(Historicals);
    }
}

This answer is not really that good, you are still doing wierd things, and it has no explination of what you were doing wrong. You need to make a new distinct object every time you add it, without all the redundancy — TheGeneral, Mar 24 '19 at 22:37

Maarten Daniels · Answer 5 · 2019-04-07T17:54:28.063

Improving on my answer and as suggested try to better explain what happens based on the comments below (that are all valid).

class Program
{
    public class HistoricValue
    {
        public string Name { get; set; }
        public DateTime Lastdate { get; set; }
        public Double Value { get; set; }
    }
    private static void Display(List<HistoricValue> cs)
    {
        Console.WriteLine();
        foreach (HistoricValue item in cs)
        {
            Console.WriteLine("{0} {1} {2} ", item.Name, item.Lastdate.ToString(), item.Value);
        }
    }
    static void Main(string[] args)
    {
        HistoricValue newValue  = new HistoricValue();
        List<HistoricValue> Historicals = new List<HistoricValue>();
        newValue.Name= "Some name 1";
        newValue.Lastdate = DateTime.Parse("2018-05-08");
        newValue.Value = 310.1;
        Historicals.Add(new HistoricValue () { Name=newValue.Name, Lastdate= newValue.Lastdate, Value = newValue.Value });
        Historicals.Add(newValue);
        Console.WriteLine("Expected output: Twice Some Name 1");
        Display(Historicals);
        newValue.Name = "Some name 2";
        newValue.Lastdate = DateTime.Parse("2018-09-09");
        newValue.Value = 210.1;
        Historicals.Add(new HistoricValue() { Name = newValue.Name, Lastdate = newValue.Lastdate, Value = newValue.Value });
        Historicals.Add(newValue);
        Console.WriteLine("\nExpected output: Twice Some Name 1 and twice somename 2");
        Display(Historicals);
        Console.WriteLine("\nReceived output: once Some name 1 and tree times somename 2");
        Console.WriteLine("\nnewValue get assigned values, what is stored in the list is the pointer to values, so item 2,3,4 will point to the same values in memory.");

        List<HistoricValue> Historicals2 = new List<HistoricValue>();
        Console.WriteLine("\nRCorrect ways to fill the list can be by using a constructor");
        Historicals2.Add(new HistoricValue() { Name = "Some name 1", Lastdate = DateTime.Parse("2018-05-08"), Value = 310.1 });
        Historicals2.Add(new HistoricValue() { Name = "Some name 2", Lastdate = DateTime.Parse("2018-06-08"), Value = 100.1 });
        Console.WriteLine("Expected output: Some Name 1 and Somename 2");
        Display(Historicals2);
        Console.WriteLine("\nOr add with specifically creating a new posistion in the list and add it.");
        newValue.Name = "Some name 3";
        newValue.Lastdate = DateTime.Parse("2018-05-08");
        newValue.Value = 310.1;
        Historicals2.Add(new HistoricValue() { Name = newValue.Name, Lastdate = newValue.Lastdate, Value = newValue.Value });
        newValue.Name = "Some name 4";
        newValue.Lastdate = DateTime.Parse("2018-09-09");
        newValue.Value = 999;
        Historicals2.Add(new HistoricValue() { Name = newValue.Name, Lastdate = newValue.Lastdate, Value = newValue.Value });
        Console.WriteLine("Expected output: Some Name 1,2,3 and 4");
        Display(Historicals2);
        Console.WriteLine("\nOr through using a loop in wich a variable is created and assiged and then stops living.");
        for( int x = 5; x<= 7; x++)
        {
            HistoricValue newValueInLoop = new HistoricValue();
            newValueInLoop.Name = "Some name " + x.ToString();
            newValueInLoop.Lastdate = DateTime.Parse("2018-09-09");
            newValueInLoop.Value = 999+x;
            Historicals2.Add(new HistoricValue() { Name = newValueInLoop.Name, Lastdate = newValueInLoop.Lastdate, Value = newValueInLoop.Value });
            //Display(Historicals2);
        }
        Console.WriteLine("Expected output: Some Name 1,2,3,4,5,6,7");
        Display(Historicals2);
        Console.WriteLine("Actually this is strange, realizing the variable only exists in the loop, yet the memory values are retainted, i hope the garbage collector works");
    }
}

Why does adding values to a collection overwrites previous items and how to simplify?

5 Answers5