Please help: Object reference not set to an instance of an object ...

[shapper]

Hello,

I have the following loop and error message:
foreach (Theme theme in Profile.Collaborator.Themes) { ...

And I get the following error:
Object reference not set to an instance of an object.

I know I get this error because Themes is null ... I just don't know
how to solve it without needing to do:
Profile.Collaborator.Themes = new List<Theme>();

So here is my code before I use the loop:
MyProfile = new ProfileLeaf(helper);

helper is an instance of ProfileHelper which includes the properties
ProfileLeaf needs.

ProfileLeaf is as follows:

public class ProfileLeaf {
public Collaborator Collaborator { get; set; }
public Contact Contact { get; set; }

public ProfileLeaf() {
this.Collaborator = new Collaborator();
this.Contact = new Contact();
}
public ProfileLeaf(ProfileHelper profile) {
// this.Collaborator = profile.Collaborator == null ?
profile.Collaborator : new Collaborator();
this.Collaborator = profile.Collaborator;
this.Contact = profile.Contact;
}
}

Note the commented line! If I use this I am able to solve the problem.
But what if Collaborator is not empty but only the Themes inside
it ...

Going on. Collaborator class:

public class Collaborator {
public string CurriculumVitae { get; set; }
public List<Theme> Themes { get; set; }

public Collaborator() {
this.Themes = new List<Theme>();
}
}

Theme is a class with two properties:

public class Theme {
public Subject Subject { get; set; }
public List<Level> Levels { get; set; }
}

Basically, I would like to integrate on my code, in each class, a way
to prevent this kind of errors.

How should I do this?

Thanks,
Miguel

 

[Jeroen Mostert]

I have the following loop and error message:
foreach (Theme theme in Profile.Collaborator.Themes) { ...

And I get the following error:
Object reference not set to an instance of an object.

I know I get this error because Themes is null ... I just don't know
how to solve it without needing to do:
Profile.Collaborator.Themes = new List<Theme>();

What's wrong with that? If there are no themes, then the collection of
themes is empty. Makes perfect sense to me.

Allowing null introduces special cases; reducing special cases is good.
Don't allow null in the first place if you don't want it.

So here is my code before I use the loop:
MyProfile = new ProfileLeaf(helper);

helper is an instance of ProfileHelper which includes the properties
ProfileLeaf needs.

ProfileLeaf is as follows:

public class ProfileLeaf {
public Collaborator Collaborator { get; set; }
public Contact Contact { get; set; }

public ProfileLeaf() {
this.Collaborator = new Collaborator();
this.Contact = new Contact();
}

This class contradicts itself. It initializes its properties with valid
references in the constructor, but then allows any client to overwrite these
with anything, including null. Is this supposed to encapsulate anything?

If "ProfileLeaf" is just the one-time combination of Collaborator and
Contact, a much better idea is to make the class immutable, thereby forcing
the references to be not null:

public class ProfileLeaf {
private readonly Collaborator collaborator;
public Collaborator Collaborator { get { return collaborator; } }

private readonly Contact contact;
public Profile Profile { get { return profile; } }

public ProfileLeaf(Collaborator collaborator, Contact contact) {
if (collaborator == null) throw new
ArgumentNullException("collaborator");
if (contact == null) throw new ArgumentNullException("contact");
}
}

Whenever you're passed a ProfileLeaf now you don't need to check if the
fields are null, because they can't be. Clients can't change a ProfileLeaf
object: they can only construct new ones. Interfaces of other objects may
need to change to accommodate this.

public class Collaborator {
public string CurriculumVitae { get; set; }
public List<Theme> Themes { get; set; }

public Collaborator() {
this.Themes = new List<Theme>();
}
}

The same problem, with the same solution. At the very least "Themes" should
always be a valid collection and it should *never* be set by clients:

public class Collaborator {
private readonly List<Theme> themes;
public IList<Theme> Themes { get { return themes; } }

public Collaborator() {
this.themes = new List<Theme>();
}

// Clients may want this for convenience
public Collaborator(IEnumerable<Theme> themes) {
this.themes = new List<Theme>(themes);
}
}

Basically, I would like to integrate on my code, in each class, a way
to prevent this kind of errors.

Applying immutability and constructor checks will go a long way, but
sometimes a check will be unavoidable. Make it clear which things cannot
ever be null and enforce this, then the remainder should hopefully be simple
enough to always check.

 

[Jeroen Mostert]

If "ProfileLeaf" is just the one-time combination of Collaborator and
Contact, a much better idea is to make the class immutable, thereby
forcing the references to be not null:

public class ProfileLeaf {
private readonly Collaborator collaborator;
public Collaborator Collaborator { get { return collaborator; } }

private readonly Contact contact;
public Profile Profile { get { return profile; } }

public ProfileLeaf(Collaborator collaborator, Contact contact) {
if (collaborator == null) throw new
ArgumentNullException("collaborator");
if (contact == null) throw new ArgumentNullException("contact");
}

Heh, I obviously forgot the crucial code for initializing the fields...
Luckily the compiler will catch this.