Adding Events to subclasses of Winparts

pirithoos posted on Tuesday, November 14, 2006

(Csla v2.1)

Hi there,

In my application there is a combobox used to allow the user to select a SubstanceBO to edit.
I have adopted Rockys design from PTracker with user controls inherited from WinPart.
Once I have added a new Substance or e.g. changed the name of a Substance during
editing it I want the combobox to immeidately reflect either the changes to the Substance.Name
(basically used as DisplayMember of the ComboBox) or to show the newly added Substance in the list to allow selecting it from the ComboBox after saving it to the DB.

My idea is to add new event to my 'SubstanceEdit' user control (inherited from Winpart)
called 'SubstanceIsSaved'. This event is raised when the SaveButton on the SubstanceEdit UC
is clicked and the SaveSubstance Sub is invoked...

    Private Sub SaveSubstance(ByVal rebind As Boolean)
        Me.SubstanceBindingSource.RaiseListChangedEvents = False
        Dim temp As Substance = mSubstance.Clone
        temp.ApplyEdit()
        Try
            mSubstance = temp.Save
            mSubstance.BeginEdit()
            If rebind Then
                Me.SubstanceBindingSource.DataSource = Nothing
                Me.SubstanceBindingSource.RaiseListChangedEvents = True
                Me.SubstanceBindingSource.DataSource = mSubstance
                ApplyAuthorizationRules()
            End If
        Catch ex As Csla.DataPortalException
            MessageBox.Show(ex.BusinessException.ToString)
        Catch ex As Exception
            MessageBox.Show(ex.ToString, "Error saving", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Finally
            Me.ButtonSave.Enabled = mSubstance.IsSavable
            Me.UndoButton.Enabled = mSubstance.IsDirty
            RaiseEvent SubstanceSaved(Me, EventArgs.Empty)
        End Try

    End Sub

Within the application's MainForm I did add a Handler to listen to this event and if fired I do
force the SubstanceListBindingSource (where the ComboBox is bound to) to freshly retrieve
all data from the DB by setting it to a new instance of SubstanceList.

#Region " WinPart handling "

    Private Sub AddWinPart(ByVal part As WinPart)
        AddHandler part.CloseWinPart, AddressOf CloseWinPart
        Dim obj As SubstanceEdit = CType(part, SubstanceEdit)
        AddHandler obj.SubstanceSaved, AddressOf ReloadComboboxesAfterSubstanceIsSaved
         Panel1.Controls.Add(part)
        ShowWinPart(part)

    End Sub
...

 Private Sub ReloadComboboxesAfterSubstanceIsSaved(ByVal sender As Object, ByVal e As EventArgs)
        Me.SubstanceListBindingSource.DataSource = SubstanceList.GetSubstanceList
    End Sub

QUESTION:

the code workes, but I wonder if I it's a legal approach or do I cause any unforeseen side-effects?

brgds,
Frank

Bayu replied on Tuesday, November 14, 2006

IMO: if it works you already have at least one strong argument in favour of your approach.

Ok, no kidding. It looks fine to me. It is entirely a UI issue, so your choice to make appropriate changes in the UI (as opposed to in the BO) is a sound one. In addition, by making use of Events you are in fact implementing a observer pattern, which is always a good pattern to keep your objects loosely coupled.

The only thing that might bother me is that in a way your WinPart subclass strongly depends on the Event actually being handled. The event is merely a 'request' than a notification. The issue with Observer patterns (or Event raising) is that you never know if their will actually be a recieving party. This is of course by design and makes them so well suited for 'notifications'.

If you find that your (and perhaps many more) view really needs to be sure there is a collaborating object that takes care of refreshing the bindingsource for your, then you might opt for a more strongly coupled approach:
- define an interface IRefreshService
- this interface declares 1 member: RefreshNow (with appropriate arguments)
- make your mainform implement this IRefreshService
- alter the constructor of your WinPart so it requires a reference to a IRefreshService instance (i.e. your mainform, but using your interface you keep it loosely coupled)
- in your SaveSubstance routine you then call the RefreshNow when needed

You asked for detailed feed-back, hence I;m giving it. By no means do I intend to criticise, I think your approach is fine as it is (hey! It works, right?), I'm hinting at a possible approvement just to be thorough. ;-)

Kind regards,
Bayu

ajj3085 replied on Tuesday, November 14, 2006

Just wanted to agree with Bayu's assertion that you can't technically be sure anyone is listening to the event, and thus shouldn't depend on someone handling it in the code you've posted.

pirithoos replied on Thursday, November 16, 2006

Thanks to both of your comments.

Now I adopted Bayus approach with the IRefreshService interface.
I do prefer Bayus appraoch as it give me a option to control what exactly to Refresh
by giving arguments to the RefreshNow method.

Thanks a lot Bayu.

brgds

Frank