Hey, folks. I was talking to a colleague at work yesterday who was reading the 3.0 e-book. He had a question, and I think he might be on to something. I did some searching and didn't see anything talking about this before...
Here is the code that's in the book (after updating it to match the errata that's been posted by Rocky):
private void ApplyButton()
{
using (StatusBusy busy = new StatusBusy("Saving..."))
{
// stop the flow of events
this.projectBindingSource.RaiseListChangedEvents = false;
this.resourcesBindingSource.RaiseListChangedEvents = false;
// commit edits in memory
UnbindBindingSource(this.resourcesBindingSource, true, false);
UnbindBindingSource(this.projectBindingSource, true, true);
try
{
// clone object and save clone
Project temp = _project.Clone();
_project = temp.Save();
// rebind the UI
this.projectBindingSource.DataSource = null;
this.resourcesBindingSource.DataSource = this.projectBindingSource;
this.projectBindingSource.DataSource = _project;
ApplyAuthorizationRules();
}
catch (Csla.DataPortalException ex)
{
MessageBox.Show(ex.BusinessException.ToString(),
"Error saving", MessageBoxButtons.OK,
MessageBoxIcon.Exclamation);
}
catch (Exception ex)
{
MessageBox.Show(ex.ToString(),
"Error Saving", MessageBoxButtons.OK,
MessageBoxIcon.Exclamation);
}
finally
{
this.projectBindingSource.RaiseListChangedEvents = true;
this.resourcesBindingSource.RaiseListChangedEvents = true;
this.projectBindingSource.ResetBindings(false);
this.resourcesBindingSource.ResetBindings(false);
}
}
}
There are a couple of things that were brought up when we reviewed this code. One, it doesn't seem to follow Rocky's "best practices" outline from the previous page. This is, honestly, something of a nit-picking question to me, but my colleague is somewhat new to CSLA, so all it's doing is confusing him.
However, more importantly to us is the question of what happens if an exception is thrown. The way this looks to us, if an exception is thrown, the original object is never re-bound to the BindingSource controls. If that's the case, then the form is pretty much unusable if something happens.
I honestly pretty much skimmed over this section, as I had already built some base forms to manage this, and as such I assumed that my code - since it appears to be working in our production code - was the same. It turns out it's not the same, which makes me wonder if I'm just that lucky... ![Embarrassed [:$]](/emoticons/emotion-10.gif)
Anyway, after discussing the situation, here is how we think the "Apply" code should look:
private void ApplyButton()
{
using (StatusBusy busy = new StatusBusy("Saving..."))
{
// stop the flow of events
this.projectBindingSource.RaiseListChangedEvents = false;
this.resourcesBindingSource.RaiseListChangedEvents = false;
// commit edits in memory
UnbindBindingSource(this.resourcesBindingSource, true, false);
UnbindBindingSource(this.projectBindingSource, true, true);
this.resourcesBindingSource.DataSource = this.projectBindingSource;
try
{
// clone object and save clone
Project temp = _project.Clone();
_project = temp.Save();
}
catch (Csla.DataPortalException ex)
{
MessageBox.Show(ex.BusinessException.ToString(),
"Error saving", MessageBoxButtons.OK,
MessageBoxIcon.Exclamation);
}
catch (Exception ex)
{
MessageBox.Show(ex.ToString(),
"Error Saving", MessageBoxButtons.OK,
MessageBoxIcon.Exclamation);
}
finally
{
// rebind the UI
this.projectBindingSource.DataSource = _project;
ApplyAuthorizationRules();
this.projectBindingSource.RaiseListChangedEvents = true;
this.resourcesBindingSource.RaiseListChangedEvents = true;
this.projectBindingSource.ResetBindings(false);
this.resourcesBindingSource.ResetBindings(false);
}
}
}
One difference is subtle, and it relates to that "best practice" comment I made earlier. There is no need for the "this.projectBindingSource.DataSource = null;" line - the "UnbindBingingSource" method does that already. And we moved the "this.resourcesBindingSource.DataSource = this.projectBindingSource;" line so it's right after the "UnbindBingingSource" calls. This matches the procedure outlined on the previous page, and may be a slightly safer way of doing things if an exception is thrown.
The other difference is that we took the re-binding code and moved it into the "finally" block. This addresses the concern we had about if an exception is thrown - since the concept of hitting an "Apply" button is that the form, and thus the BO, is still usable, the re-bind should happen even if an exception happens on the Save (which is where it's most likely to occur.) I realize that the most likely course of action on an exception is that the user will have to close the form anyway, thus discarding any changes. But in this version, the form is still usable, and you shouldn't get the kinds of failures we think would happen in the original version.
Thoughts? Opinions? Are we missing something? Since I'm looking at updating my code to match this, I'd like to know if I've missed the boat somewhere...
Thanks!
Copyright (c) Marimer LLC