I'm working on a school project in Windows Forms where we're making an animal registry. I have an Animal class where the animal object is created with the properties Id, Name, Age, Gender, and Friendly. I also have an AnimalManager class that handles a list and through which more animals can be added. At runtime, when having filled out the appropriate boxes with information and pressed the Add Animal-button, the information should be added to a row in a listview (the important part of the task. Everything was peaceful when I could just display it in a listbox).
When I printed my information to a listbox I simply sent a string with the appropriate information through public string AnimalInformation()
from the Animal class to the AnimalManager that compiled an array of it and sent it to Form1 to be printed. Since I want to display it in a listview, I figured I should make AnimalInformation()
into an array instead, so that I get a 2D array after passing through the AnimalManager, much like it will be displayed when printed. However, I get the exception message System.NullReferenceException: 'Object reference not set to an instance of an object.' in my AnimalManager when running the program and trying to add the animal.
This is the method in the Animal class:
public string[] AnimalInformation()
{
string[] strOut = { id, name, age.ToString(), gender.ToString(), FriendlyStr()};
return strOut;
}
This is the method in the AnimalManager class:
public string[][] GetAnimalInfoArray()
{
string[][] animals = new string[animalList.Count][];
for(int i = 0; i < animalList.Count; i )
{
Animal animal = animalList[i];
for(int j = 0; j < animal.CountAnimalInfo(); j )
{
string[] info = animal.AnimalInformation();
animals[i][j] = info[j];
}//here comes the exception
}
return animals;
}
And since I am not too familiar with listview (read: not at all), here's the loop I wrote for printing it all into the listview in my UpdateGUI()
method, in case this is what is causing the trouble:
if (manager.Count() > 0)
{
foreach(string[] row in manager.GetAnimalInfoArray())
{
foreach(string item in row)
lvwAnimals.Items.Add(item);
}
}
I've been at this for hours and don't know what's up or down anymore. Am I even on the right track? Do I have to rework this completely? What am I doing wrong to get an error message after that curly bracket?
CodePudding user response:
The first problem is that you're creating an array of arrays but not allocating the arrays in the array... which is mildly confusing, but this is the line:
string[][] animals = new string[animalList.Count][];
animals
is an array of string[]
. At this point, each element in the animals
array is null, not an array. The point where you're having a problem is this:
animals[i][j] = info[j];
At this point animals[i]
is uninitialized (null
), so there is no array for you to index with j
.
There are a number of solutions to the problem, but the most direct method is to simply assign the result of calling AnimalInformation
to the array slot and be done with it:
public string[][] GetAnimalInfoArray()
{
string[][] animals = new string[animalList.Count][];
for(int i = 0; i < animalList.Count; i )
animals[i] = animalList[i].AnimalInformation();
return animals;
}
If you're OK with using LINQ there's an even simpler option: ToArray()
.
public string[][] GetAnimalInfoArray()
=> animalList.Select(a => a.AnimalInformation()).ToArray();
A couple of notes on the rest of the code...
Your inner loop calls CountAnimalInfo()
and AnimalInformation()
for each item in the information array. That's 5 times you're calling those two methods, creating at least one array each time you do (and I'm assuming you're not calling AnimalInformation()
from inside CountAnimalInfo()
, if you are then that's two arrays per item). It would be better to simply call it once and use the resultant array for the loop, if you have to loop at all.
for (int i = 0; i < animalList.Count; i )
{
var info = animalList[i].AnimalInformation();
animals[i] = new string[info.Length];
for (int j = 0; j < info.Length; j )
animals[i][j] = info[j];
}
You're spreading what appears to be display code across several methods and classes. While there are times when it's useful to be able to override things in sub-classes, it's generally preferable (in my experience at least) to have the display code in one place: a single method or tightly coupled group of methods in a single class. An Animal
doesn't need to know how it is being displayed, it just needs to present properties that other code can use. Your AnimalManager
class probably doesn't need to act as go-between for your display code either, it's up to the display code to figure out how things should look. Try this:
foreach (var animal in manager.animalList)
{
lvwAnimals.Items.Add(animal.id);
lvwAnimals.Items.Add(animal.name);
lvwAnimals.Items.Add(animal.age.ToString());
lvwAnimals.Items.Add(animal.gender.ToString());
lvwAnimals.Items.Add(animal.FriendlyStr());
}
This way you've separated the 'display' concern out from the non-display classes, letting them focus more on doing their own job. And extra bonus, it's a lot less lines of code to debug later.
CodePudding user response:
I know this doesnt directly answer , JeremyLakeMan comments about why you have a problem at the moment. But here is what I would do
Given an IEnumerable of Animals called animalList (which you already have)
Assuming Animal Looks Like this
public class Animal{
public string ID;
public String Name;
public int Age;
}
Now
foreach(var animal in animals)
{
lvwAnimals.Items.Add(animal.ID);
lvwAnimals.Items.Add(animal.Name);
lvwAnimals.Items.Add(animal.Age.ToString());
}
thats it, the whole GetAnimalInfoArray is not needed