HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Disposable Heroes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
disposableheroesstackoverflow

Problem

I had a bit of an issue with my last piece of code, having to do with cleaning up resources. I needed a way to ensure the database connection was always properly closed, even if there still were instances of an IPresenter or IRepository still floating around.


IDisposable class module

Option Explicit

Public Sub Dispose()
End Sub


So I made a minor tweak to my UnitOfWork class, and implemented this IDisposable interface:


UnitOfWork class module

```
Option Explicit

Private Const CONNECTION_STRING As String = "DRIVER={MySQL ODBC 5.1 Driver};UID=;PWD=;SERVER=;DATABASE=;PORT=;"

Private repositories As New Dictionary
Private adoConnection As New ADODB.Connection

Private disposed As Boolean

Implements IUnitOfWork
Implements IDisposable

Private Sub Class_Initialize()
adoConnection.ConnectionString = CONNECTION_STRING
adoConnection.Open
adoConnection.BeginTrans
End Sub

Private Sub Class_Terminate()
If Not disposed Then Dispose
End Sub

Public Sub Dispose()
Set repositories = Nothing
If Not adoConnection Is Nothing Then
If adoConnection.State = adStateOpen Then
adoConnection.RollbackTrans 'rollback any uncommitted changes
adoConnection.Close
End If
Set adoConnection = Nothing
End If
disposed = True
End Sub

Private Sub IDisposable_Dispose()
If Not disposed Then Dispose
End Sub

Public Sub AddRepository(ByVal key As String, ByRef repo As IRepository)
repo.SetConnection adoConnection
repositories.Add key, repo
End Sub

Public Property Get Repository(ByVal key As String) As IRepository
Set Repository = repositories(key)
End Property

Public Sub Commit()
adoConnection.CommitTrans
adoConnection.BeginTrans
End Sub

Public Sub Rollback()
adoConnection.RollbackTrans
adoConnection.BeginTrans
End Sub

Private Sub IUnitOfWork_AddRepository(ByVal key As String, ByRef repo As IRepository)
AddRepository key, repo
End Sub

Private Sub IUnitOfWor

Solution

VBA suffers from a lack of proper inheritance and we have to make up for it with composition. In a sensible language, like Ruby (hardy har har) or C#, I would create a PresenterBase class that implements IDisposable and Presenter would then inherit it's Dispose method. Like I said, that's not an option.

So, have you cleaned it up, or made a mess of it? Yes and yes. You can now cleanly dispose of your presenter, but it's not so clean underneath.

Interfaces shouldn't implement other interfaces, as you've noticed it's nonsensical. Remove all of the dispose code from IPresenter. I'm confused by your assertion that Implementing more than one interface is a pain. It's exactly as it should be.

What you should do is create a PresenterBase class that is responsible for defining common code. In this case, the Dispose method. Here is an extremely stripped down example. IDisposable and IPresenter should remain the same (with the exception of removing the disposable code from IPresenter).

PresenterBase

Private Type tPresenter
    MasterId As Long
    Repository As IRepository
    DetailsPresenter As IPresenter
    View As IView
End Type

Private this As tPresenter

Public Sub Dispose()
    IDisposable_Dispose
End Sub

Private Sub IDisposable_Dispose()
    If Not View Is Nothing Then Unload View
    If Not this.UnitOfWork Is Nothing Then this.UnitOfWork.Dispose
    If Not this.DetailsPresenter Is Nothing Then this.DetailsPresenter.Dispose
    Set this.UnitOfWork = Nothing
    Set this.View = Nothing
    Set this.DetailsPresenter = Nothing
End Sub


Presenter

Implements IPresenter
Implements IDisposable

Private base As PresenterBase

Public Property Get UnitOfWork()
    base.UnitOfWork
End Property

'etc

Public Sub Dispose()
    IDisposable_Dispose
End Sub

Private Sub IDisposable_Dispose()
    base.Dispose
End Sub

Private Sub Class_Initialize()
    Set base = New PresenterBase
End Sub

Code Snippets

Private Type tPresenter
    MasterId As Long
    Repository As IRepository
    DetailsPresenter As IPresenter
    View As IView
End Type

Private this As tPresenter

Public Sub Dispose()
    IDisposable_Dispose
End Sub

Private Sub IDisposable_Dispose()
    If Not View Is Nothing Then Unload View
    If Not this.UnitOfWork Is Nothing Then this.UnitOfWork.Dispose
    If Not this.DetailsPresenter Is Nothing Then this.DetailsPresenter.Dispose
    Set this.UnitOfWork = Nothing
    Set this.View = Nothing
    Set this.DetailsPresenter = Nothing
End Sub
Implements IPresenter
Implements IDisposable

Private base As PresenterBase

Public Property Get UnitOfWork()
    base.UnitOfWork
End Property

'etc

Public Sub Dispose()
    IDisposable_Dispose
End Sub

Private Sub IDisposable_Dispose()
    base.Dispose
End Sub

Private Sub Class_Initialize()
    Set base = New PresenterBase
End Sub

Context

StackExchange Code Review Q#61034, answer score: 3

Revisions (0)

No revisions yet.