0

I have the following:

public class Animal
    public int currentPopulation 
    public string name

   public Animal(int currentPopulation, string name){
       this.currentPopulation = currentPopulation;
       this.name = name;
   }

In another class I have:

public class MainClass

   <List><Animal>animalList
   ...

   lion = newAnimal(100, "Lion");
   cat = newAnimal(20, "Cat");
   dog = newAnimal(40, "Dog")

   animalList.add(lion);
   animalList.add(cat);
   animalList.add(dog);

Every so often I have to fetch new data from a server and update the animal property, currentPopulation in the MainClass. Currently I'm doing this by the following:

public void UpdatePopulations(int lionPopulation, int catPopulation, int dogPopulation)

foreach(var item in animalList.where(n=>n.name=="Lion")){
   item.currentPopulation = lionPopulation;
}
... and the same for the cat and dog. 

I feel like my solution is bulky and I'm wondering if there is a cleaner way to update the objects from the list.

lost9123193
  • 10,460
  • 26
  • 73
  • 113
  • 2
    Rather than using a list, why not use a dictionary, where the key is the animal name? – Dragonthoughts Apr 25 '18 at 05:38
  • @Dragonthoughts The code I have is simplified but essentially I have a slide show and a timer that shows information for each object in the list. So I thought it list might be easier to do it. – lost9123193 Apr 25 '18 at 05:41
  • Do you want to update just an animal or all animals in the list? Don't you want to have a parameter for each name instead of hardcoding the `"Lion"`? – kurakura88 Apr 25 '18 at 05:47
  • 1
    @kurakura88 i want to update all the animals hence why I added a field called lions – lost9123193 Apr 25 '18 at 05:49
  • As already answered, use a dictionary, and not only during the update, as re-creating a dictionary for each update creates unnecessary overhead. Also add an `int` or `enum` (which is an int) Id property to your class, avoid strings as Ids, as these tend to be [less efficient](https://stackoverflow.com/questions/5743474/key-performance-for-a-dictionary). This will make it easier to identify the class and very easy to look up in the dictionary. – r41n Apr 25 '18 at 07:15

4 Answers4

1

When working with list (which is a dynamic container) there is no way to find element without iterating over it. One way you can make it more efficient is updating your List in one path and not using LINQ to find elements. Something like this

foreach(vat animal in animalList) 
{
    if (animal.name == "Lion")
        animal.currentPopulation = ...
    else if (animal.name == "...")
        animal.currentPopulation = ...

}

I'd suggest you use different data container than list. Dictionary<string, Animal> animals will serve you better because than you can use animal name as update keys.

JobNick
  • 473
  • 1
  • 6
  • 18
1
public void UpdatePopulations(Dictionary<string, int> populations)
{
    foreach(var population in populations)
        foreach(var animal in animalList.Where(x => x.name == population.Key))
            animal.currentPopulation = population.Value;        
}

Usage:

variableName.UpdatePopulations(new Dictionary<string, int> {
   ["Lion"] = 1000,
   ["Cat"] = 2000,
   ["Dog"] = 3000
});
Slava Utesinov
  • 13,410
  • 2
  • 19
  • 26
1

You can also differentiate with the help of 'enum' as shown below but you need to iterate through list to identify animal.

public enum AnimalName
{
    Lion,
    Cat,
    Dog
}

public class Animal
{
    public AnimalName Name { get; set; }

    public int Population { get; set; }

    public Animal(AnimalName name, int population)
    {
        Name = name;
        Population = population;
    }
}

public void UpdatePopulations(int lionPopulation, int catPopulation, int dogPopulation)
    {
        foreach (var animal in animals)
        {
            switch (animal.Name)
            {
                case AnimalName.Lion:

                    animal.Population = lionPopulation;
                    break;

                case AnimalName.Cat:

                    animal.Population = catPopulation;
                    break;

                case AnimalName.Dog:

                    animal.Population = dogPopulation;
                    break;
            }
        }
    }
Advait Baxi
  • 1,530
  • 14
  • 15
1

There is almost no benefit to convert the animalList to Dictionary, if you are going to update all of the animals anyway. Some improvement that I can think of is to accept IEnumerable instead, so you can freely update certain or all animals.

public void UpdatePopulations(IEnumerable<Animal> newAnimals)
{
    var dictionary = newAnimals.ToDictionary<string, int>(a=>a.Name, a=>a.currentPopulation); // convert to dictionary, so that we have O(1) lookup during the search later. This process itself is O(n)
    foreach(var animal in animalList) // this will be O(n)
    {
        if(dictionary.ContainsKey(animal.Name))
        {
            animal.currentPopulation = dictionary[animal.Name].currentPopulation;
        }
    }
}
kurakura88
  • 2,185
  • 2
  • 12
  • 18