Skip to main content
Tweeted twitter.com/StackCodeReview/status/1253111382112972800
added IDbConnection for additional context
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469

Can anything be improved? Did I forget any tests? I write in README.md that the abstraction is leaky on purpose in order to retain the full flexibility of the ADODB library, ...but maybeI have to admit it couldlooks pretty airtight at that level.

At the IDbConnection level though...

...Wrinkles appear. It's almost as if I needed another interface to separate the internal API from the public API!

IDbConnection

Here I'm wrapping an ADODB.Connection - I need to expose the ADODB.Connection object somehow, in order to be tightened some more without sacrificingable to create commands off that flexibility?connection.

VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "IDbConnection"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = True
Attribute VB_Description = "Represents an object that wraps an active ADODB connection."
'@Folder("SecureADODB.DbConnection.Abstract")
'@ModuleDescription("Represents an object that wraps an active ADODB connection.")
'@Exposed
'@Interface
Option Explicit

'@Description("Gets the wrapped ADODB connection.")
Public Property Get AdoConnection() As ADODB.Connection
Attribute AdoConnection.VB_Description = "Gets the wrapped ADODB connection."
End Property

'@Description("Gets the state of the wrapped ADODB connection.")
Public Property Get State() As ADODB.ObjectStateEnum
Attribute State.VB_Description = "Gets the state of the wrapped ADODB connection."
End Property

'@Description("Creates an ADODB.Command that uses the wrapped connection")
Public Function CreateCommand(ByVal commandType As ADODB.CommandTypeEnum, ByVal sql As String) As ADODB.Command
Attribute CreateCommand.VB_Description = "Creates an ADODB.Command that uses the wrapped connection"
End Function

'@Description("Returns the object itself. Useful to retrieve the With object variable in a With block.")
Public Property Get Self() As IDbConnection
Attribute Self.VB_Description = "Returns the object itself. Useful to retrieve the With object variable in a With block."
End Property

Public Sub BeginTransaction()
End Sub

Public Sub CommitTransaction()
End Sub

Public Sub RollbackTransaction()
End Sub

Can anything be improved? Did I forget any tests? I write in README.md that the abstraction is leaky on purpose in order to retain the full flexibility of the ADODB library, ...but maybe it could be tightened some more without sacrificing that flexibility?

Can anything be improved? Did I forget any tests? I write in README.md that the abstraction is leaky on purpose in order to retain the full flexibility of the ADODB library, ...but I have to admit it looks pretty airtight at that level.

At the IDbConnection level though...

...Wrinkles appear. It's almost as if I needed another interface to separate the internal API from the public API!

IDbConnection

Here I'm wrapping an ADODB.Connection - I need to expose the ADODB.Connection object somehow, in order to be able to create commands off that connection.

VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "IDbConnection"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = True
Attribute VB_Description = "Represents an object that wraps an active ADODB connection."
'@Folder("SecureADODB.DbConnection.Abstract")
'@ModuleDescription("Represents an object that wraps an active ADODB connection.")
'@Exposed
'@Interface
Option Explicit

'@Description("Gets the wrapped ADODB connection.")
Public Property Get AdoConnection() As ADODB.Connection
Attribute AdoConnection.VB_Description = "Gets the wrapped ADODB connection."
End Property

'@Description("Gets the state of the wrapped ADODB connection.")
Public Property Get State() As ADODB.ObjectStateEnum
Attribute State.VB_Description = "Gets the state of the wrapped ADODB connection."
End Property

'@Description("Creates an ADODB.Command that uses the wrapped connection")
Public Function CreateCommand(ByVal commandType As ADODB.CommandTypeEnum, ByVal sql As String) As ADODB.Command
Attribute CreateCommand.VB_Description = "Creates an ADODB.Command that uses the wrapped connection"
End Function

'@Description("Returns the object itself. Useful to retrieve the With object variable in a With block.")
Public Property Get Self() As IDbConnection
Attribute Self.VB_Description = "Returns the object itself. Useful to retrieve the With object variable in a With block."
End Property

Public Sub BeginTransaction()
End Sub

Public Sub CommitTransaction()
End Sub

Public Sub RollbackTransaction()
End Sub
added 37 characters in body
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469

Can anything be improved? Did I forget any tests? I write in README.md that the abstraction is leaky on purpose in order to retain the full flexibility of the ADODB library, ...but maybe it could be tightened some more without sacrificing that flexibility?


Best Practices

I've given a bit of thought about the various ways the API could be used & misused (probably something to do with the leaky abstractions!), and came up with these "best practices" usage guidelines:

  • DO hold the object reference in a With block. (e.g. With New UnitOfWork.FromConnectionString(...))
  • DO have an active On Error statement to graciously handle any errors.
  • DO commit or rollback the transaction explicitly in the scope that owns the IUnitOfWork object.
  • CONSIDER passing IDbCommand as an argument to other scopes.
  • AVOID passing IUnitOfWork as a parameter to another object or procedure.
  • AVOID accidentally re-entering a With block from an error-handling subroutine (i.e. avoid Resume, Resume [label], and Resume Next in these subroutines). If there was an error, execution jumped out of the With block that held the references, and the transaction was rolled back and the connection is already closed: there's nothing left to clean up.

I think that's pretty consistent with the implementation, but did I miss anything important?

Can anything be improved? Did I forget any tests?

Can anything be improved? Did I forget any tests? I write in README.md that the abstraction is leaky on purpose in order to retain the full flexibility of the ADODB library, ...but maybe it could be tightened some more without sacrificing that flexibility?


Best Practices

I've given a bit of thought about the various ways the API could be used & misused (probably something to do with the leaky abstractions!), and came up with these "best practices" usage guidelines:

  • DO hold the object reference in a With block. (e.g. With New UnitOfWork.FromConnectionString(...))
  • DO have an active On Error statement to graciously handle any errors.
  • DO commit or rollback the transaction explicitly in the scope that owns the IUnitOfWork object.
  • CONSIDER passing IDbCommand as an argument to other scopes.
  • AVOID passing IUnitOfWork as a parameter to another object or procedure.
  • AVOID accidentally re-entering a With block from an error-handling subroutine (i.e. avoid Resume, Resume [label], and Resume Next in these subroutines). If there was an error, execution jumped out of the With block that held the references, and the transaction was rolled back and the connection is already closed: there's nothing left to clean up.

I think that's pretty consistent with the implementation, but did I miss anything important?

added 37 characters in body
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469
  
VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "IUnitOfWork"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = True
Attribute VB_Description = "Represents an object encapsulating a database transaction."
'@Folder("SecureADODB.UnitOfWork")
'@ModuleDescription("Represents an object encapsulating a database transaction.")
'@Interface
'@Exposed
Option Explicit

'@Description("Commits the transaction.")
Public Sub Commit()
Attribute Commit.VB_Description = "Commits the transaction."
End Sub

'@Description("Rolls back the transaction.")
Public Sub Rollback()
Attribute Rollback.VB_Description = "Rolls back the transaction."
End Sub

'@Description("Creates a new command to execute as part of the transaction.")
'@Ignore ShadowedDeclaration: false positive here, this is an abstract @Interface class
Public Function Command() As IDbCommand
Attribute Command.VB_Description = "Creates a new command to execute as part of the transaction."
End Function
  
VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "IUnitOfWork"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = True
Attribute VB_Description = "Represents an object encapsulating a database transaction."
'@Folder("SecureADODB.UnitOfWork")
'@ModuleDescription("Represents an object encapsulating a database transaction.")
'@Interface
'@Exposed
Option Explicit

'@Description("Commits the transaction.")
Public Sub Commit()
Attribute Commit.VB_Description = "Commits the transaction."
End Sub

'@Description("Rolls back the transaction.")
Public Sub Rollback()
Attribute Rollback.VB_Description = "Rolls back the transaction."
End Sub

'@Description("Creates a new command to execute as part of the transaction.")
'@Ignore ShadowedDeclaration: false positive here,
Public Function Command() As IDbCommand
Attribute Command.VB_Description = "Creates a new command to execute as part of the transaction."
End Function
  
VERSION 1.0 CLASS
BEGIN
  MultiUse = -1  'True
END
Attribute VB_Name = "IUnitOfWork"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = True
Attribute VB_Description = "Represents an object encapsulating a database transaction."
'@Folder("SecureADODB.UnitOfWork")
'@ModuleDescription("Represents an object encapsulating a database transaction.")
'@Interface
'@Exposed
Option Explicit

'@Description("Commits the transaction.")
Public Sub Commit()
Attribute Commit.VB_Description = "Commits the transaction."
End Sub

'@Description("Rolls back the transaction.")
Public Sub Rollback()
Attribute Rollback.VB_Description = "Rolls back the transaction."
End Sub

'@Description("Creates a new command to execute as part of the transaction.")
'@Ignore ShadowedDeclaration: false positive here, this is an abstract @Interface class
Public Function Command() As IDbCommand
Attribute Command.VB_Description = "Creates a new command to execute as part of the transaction."
End Function
Source Link
Mathieu Guindon
  • 75.6k
  • 18
  • 195
  • 469
Loading