Link to home
Start Free TrialLog in
Avatar of emi_sastra
emi_sastra

asked on

Closing a Class

Hi All,

I have below code :

    Private Function Check_Data_Ok() As Boolean

        If Me.txtNoTransaksi.Text.Trim = "" Then
            MsgBox("Nomor Transaksi Harus Diisi ...!", MsgBoxStyle.Information, Me.Text)
            txtNoTransaksi.Focus()

            Return False
        End If

        If Me.txtCatatan.Text.Trim = "" Then
            MsgBox("Catatan Harus Diisi ...!", MsgBoxStyle.Information, Me.Text)
            txtCatatan.Focus()
            Return False
        End If

        Dim clsPublicStoredProcedure As New clsPublicStoredProcedure

        Try

            If Not clsPublicStoredProcedure.Is_Print_Transaction_Exist(Me.txtNoTransaksi.Text.Trim, "") Then
                MsgBox("Nomor Transaksi Tidak Ditemukan ...!", MsgBoxStyle.Information, Me.Text)
                Return False
            End If

        Catch ex As Exception
            MsgBox(ex.ToString, MsgBoxStyle.Information, Me.Text)
        End Try

        clsPublicStoredProcedure = Nothing

        Return True

    End Function

How to always set  clsPublicStoredProcedure   = Nothing what ever the scenario ? Because there is return code there.

Thank you.
Avatar of Nitin Sontakke
Nitin Sontakke
Flag of India image

May be you can put it in Finally block which (I believe) is guaranteed to execute every time.

Furthermore, not sure why do you want to explicitly do it as all local variables are automatically garbage collected once they are out of scope.

Unless you have some real special case for this class which needs to be explicitly taken care of, which you haven't mentioned.

Any normal / usual class, I haven't particularly been worried about disposing.
In addition to Nitin's post: You should rely on garbage collection.

Thus to your question: You only need to explicitly set an object reference to nothing, if your code stays for long time in the same scope. In your case this is not necessary, cause you're exiting your function Check_Data_Ok() immediately after using clsPublicStoredProcedure.Is_Print_Transaction_Exist(). Thus it is automatically marked as unused.

Big exception is when using external resources. Then your class should implement the IDisposable pattern (remember, all classes implementing it should be used by using the using clause) or have an explicit Dispose() call in the correct scope.

p.s. clean code principles say that  a function should have only one point of return. Thus I would write the function as follows:

Private Function Check_Data_Ok() As Boolean

    Try
        If Me.txtNoTransaksi.Text.Trim = "" Then
            MsgBox("Nomor Transaksi Harus Diisi ...!", MsgBoxStyle.Information, Me.Text)
            txtNoTransaksi.Focus()
            Return False
        ElseIf Me.txtCatatan.Text.Trim = "" Then
            MsgBox("Catatan Harus Diisi ...!", MsgBoxStyle.Information, Me.Text)
            txtCatatan.Focus()
            Return False
        Else
            Dim clsPublicStoredProcedure As New clsPublicStoredProcedure
            If Not clsPublicStoredProcedure.Is_Print_Transaction_Exist(Me.txtNoTransaksi.Text.Trim, "") Then
                MsgBox("Nomor Transaksi Tidak Ditemukan ...!", MsgBoxStyle.Information, Me.Text)
                Return False
            Else
                Return True
            End If
        End If
    Catch ex As Exception
        MsgBox(ex.ToString, MsgBoxStyle.Information, Me.Text)
    End Try

Open in new window

Or using a real single point of return:

Private Function Check_Data_Ok() As Boolean

    Dim Result As Boolean = True
    Try
        If Me.txtNoTransaksi.Text.Trim = "" Then
            MsgBox("Nomor Transaksi Harus Diisi ...!", MsgBoxStyle.Information, Me.Text)
            txtNoTransaksi.Focus()
            Result = False
        ElseIf Me.txtCatatan.Text.Trim = "" Then
            MsgBox("Catatan Harus Diisi ...!", MsgBoxStyle.Information, Me.Text)
            txtCatatan.Focus()
            Result = False
        Else
            Dim clsPublicStoredProcedure As New clsPublicStoredProcedure
            If Not clsPublicStoredProcedure.Is_Print_Transaction_Exist(Me.txtNoTransaksi.Text.Trim, "") Then
                MsgBox("Nomor Transaksi Tidak Ditemukan ...!", MsgBoxStyle.Information, Me.Text)
                Result = False
            End If
        End If
    Catch ex As Exception
        MsgBox(ex.ToString, MsgBoxStyle.Information, Me.Text)
    End Try

    Return Result

End Function

Open in new window


p.s. when the general usage pattern of your class clsPublicStoredProcedure is as shown in your sample, then you should consider making it a shared function in your class.
Avatar of emi_sastra
emi_sastra

ASKER

Hi Nitin,

- May be you can put it in Finally block which (I believe) is guaranteed to execute every time.

   Try

            If Not clsPublicStoredProcedure.Is_Print_Transaction_Exist(Me.txtNoTransaksi.Text.Trim, "") Then
                MsgBox("Nomor Transaksi Tidak Ditemukan ...!", MsgBoxStyle.Information, Me.Text)
                Return False  <------
            End If

        Catch ex As Exception
            MsgBox(ex.ToString, MsgBoxStyle.Information, Me.Text)

        Finally
             clsPublicStoredProcedure = Nothing
        End Try

At return false, it also execute the Finally ?

Thank you.
ASKER CERTIFIED SOLUTION
Avatar of Nitin Sontakke
Nitin Sontakke
Flag of India image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Hi All,

Thank you very much for your help.
So, finally, you are convinced!
Yes. Thank you.
Not sure if you got it, but my comment was intended to be a pun.
- Not sure if you got it, but my comment was intended to be a pun.
I am sorry for late provide points.

Thank you.
And just for clarity:
Using finally in your function to set clsPublicStoredProcedure = Nothing is not necessary, cause you are immediately leaving the function. Thus clsPublicStoredProcedure is marked as unused automatically. And there is no difference in how the instances is handled by GC in both cases.
@ste5an,

That's precisely what I tried to convey in my very first comment:

Furthermore, not sure why do you want to explicitly do it as all local variables are automatically garbage collected once they are out of scope.
Hi All,

I think I should use Public shared.
It is more simple to code.

Is it right ?

Thank you.
I think I should use Public shared. It is more simple to code. Is it right ?
Given your small code sample, yes, then it is (maybe) lesser code and is simpler to consume. And the consuming part was your question. Thus make it a shared function as I already recommended.
Yes. I do understand now.

Thank you.