Ayende @ Rahien

It's a girl

If stupidity was money, I would be rich

Here is a crash report that I got from NH Prof error reporting:

System.NullReferenceException: Object reference not set to an instance of an object.
  at HibernatingRhinos.NHibernate.Profiler.Client.Model.Sessions.RecentStatements.set_SelectedStatement(IStatementModel value)
  at HibernatingRhinos.NHibernate.Profiler.Client.Model.Sessions.RecentStatements.Clear()
  at HibernatingRhinos.NHibernate.Profiler.Client.Model.SessionPresenter.Clear()
  at HibernatingRhinos.NHibernate.Profiler.Client.Model.ApplicationModel.Clear()
  at HibernatingRhinos.NHibernate.Profiler.Client.Commands.Clear.Execute(Object parameter)

This seems to be pretty standard, isn’t it? Let us look at SelectedStatement:

image

There isn’t a possibility of a null reference exception, even the RaisePropertyChanged is safe from NRE.

So, what was going on? I started dismissing this as a ghost story, but I got several issues of those. Then I remember that I am not the only on working on the profiler (well, that is a lie, I got a ‘someone else updated this file’ when I tried to commit) and updated. Here is the updated version:

image

And there is was, right on the third line in the set.

Lesson learned? Don’t be stupid, svn update.

Comments

Rob
06/07/2009 04:51 PM by
Rob

Oops...

anon
06/07/2009 06:43 PM by
anon

Wouldn't the following raise and exception (presuming you're not checking the result to the value returned from the SelectedStatement property in the calling code)?

someObject.SelectedStatement = null; //Property not changed, assuming it was init'ed to null

IStatementModel sm = object.selectedStatement; //Get Property value (which is null)

sm.someCall ;//Use sm without checking for null - NullReferenceException thrown

Peter Morris
06/07/2009 06:49 PM by
Peter Morris

Rather than a foreach loop wouldn't it be better to do something like this?

set

{

if (selectedValue == value)

return;

if (selectedValue != null)

selectedValue.IsSelected = false;

selectedValue = value;

if (selectedValue != null)

selectedValue.IsSelected = true;

RaisePropertyChanged("SelectedValue");

}

Bill Barry
06/07/2009 07:23 PM by
Bill Barry

This was the primary reason I had for switching to a dvcs. Given two machines:

on 1:

commit revision 15

build

on 2:

update to revision 15

build

The fact that these two builds are not always the same is mildly unnerving (given svn, cvs or tfs). It also makes bugs like these far harder to find than they should be.

Ayende Rahien
06/07/2009 07:26 PM by
Ayende Rahien

Peter,

There may be more than one selected statement

Ayende Rahien
06/07/2009 07:29 PM by
Ayende Rahien

Bill,

The problem was that I didn't update my version of the source, there was no other problem.

I forgot to update and was not running on the same version

Peter Morris
06/08/2009 07:39 AM by
Peter Morris

I think moving the selected state up to a containing class would speed things up because it would let you use a HashSet <t instead of looping through all items (which may or may not be selected).

private HashSet <istatementmodel SelectedItems;

public bool IsSelected(IStatementModel item)

{

return SelectedItems.Contains(item);

}

pubic void Select(IStatementModel item)

{

SelectedItems.Add(item);

}

pubic void Deselect(IStatementModel item)

{

SelectedItems.Remove(item);

}

public void SetSingleSelected(IStatementModel item)

{

SelectedItems.Clear();

Select(item);

}

Pete

Comments have been closed on this topic.