CODEWITHSUNDEEP
vb.netexcel
user1283776
user1283776

Reputation: 21764

How can I test if function returns a successfully created object?

First, CreateApp() calls NewAppSheet().

Second, NewAppSheet() returns an object.

Third, CreateApp() tests whether the object creation was successful.

I find myself in this kind of situation often. What is a good practice for this situation? What are the nuances in this situation (for example: does it matter if the object was never created successfully or if it just points to nothing?)?

The code below is my best attempt at elegance. Unfortunately the 'Else' in CreateApp() never happens, so my test does not work.

   Sub CreateApp()

        Dim wks As Excel.Worksheet = NewAppSheet()
        If Not wks Is Nothing Then
            WriteAppText(wks)
        Else
            MessageBox.Show("This never happens")
        End If

    End Sub

    Function NewAppSheet() As Excel.Worksheet
        Dim sMessage As String = "Message"
        Dim sTitle As String = "Title"
        Dim sDefaultValue As String = ""
        Dim sValue As String
        Dim wks As Excel.Worksheet

        ' Display message, title, and default value
        sValue = InputBox(sMessage, sTitle, sDefaultValue)

        wks = CType(AddinModule.CurrentInstance, TimeTracker.AddinModule).ExcelApp.ActiveWorkbook.Worksheets.Add()

        Try
            wks.Name = sValue
        Catch
            wks.Delete()
            MessageBox.Show("A worksheet with that name already exists. Please type a different name next time.")
        End Try

        NewAppSheet = wks

    End Function

I am aware that I could create an extra variable called bSuccessful and use it in my test. But I figure there must be a better practice that better programmers use, so I'm asking you guys?

Upvotes: 0

Views: 276

Answers (2)

Jason Faulkner
Jason Faulkner

Reputation: 6558

You are never hitting your Else statement because NewAppSheet never returns Nothing:

Try
    wks.Name = sValue
Catch
    ' This will delete the sheet.
    ' It will NOT set wks to Nothing.
    wks.Delete()
    MessageBox.Show("A worksheet with that name already exists. Please type a different name next time.")
    ' Explicitly set wks object to nothing.
    wks = Nothing
End Try

' Use Return instead of assigning to the function name.
Return wks

As noted in the code snippet above, it is generally good practice to use Return in your VB.NET functions instead of assigning the value to the function name (e.g. NewAppSheet = wks) which is how it is done in VBA.

Since you are soliciting feedback as well: Procedural wise, your code and methodology look fine to me. It was clear what the intent of the code was and I found it easy to follow.

Upvotes: 2

dwilliss
dwilliss

Reputation: 924

Just have NewAppSheet return Nothing from the Catch block. 

    Try
        wks.Name = sValue
    Catch
        wks.Delete()
        MessageBox.Show("A worksheet with that name already exists. Please type a different name next time.")
        Return Nothing
    End Try

Upvotes: 2

Related Questions