Checking if an array is initialized and allocated in VB.NET

Question

I am creating an add-in for a piece of software our company uses, so I'm working pretty extensively with that program's API. There are many COM object members which return or contain an array that I need to work with. Sometimes functions will return an uninitialized array (Null or "Nothing" return) or sometimes they may return an initialized array that is not allocated. My goal is to eliminate redundant code in the form of If/Else blocks before using each array, or Try/Catch blocks to handle exceptions thrown by the array being un-initialized or un-allocated.

Note: I'm not trying to validate any data here - that can be done in the calling code or in a function or whatever - I'm simply looking to determine if an array is in a usable state (e.g. initialized and allocated with at least one member)

I've created a member extension for arrays to check if the array is both initialized and allocated.

I'd like to get some input on whether or not this is a well designed solution or if it's total bupkis, if I'm missing anything else I should be checking for, and if improvements can be made in terms of robustness, efficiency, or general best practices; I would like to make it as fast as possible for dealing with large arrays.

Imports System.Runtime.CompilerServices

Module Module1
#Region "Extension Methods"
    <Extension()> 
    Public Function IsAllocated(ByVal inArray As System.Array) As Boolean
        Dim FlagEx As Boolean = True
        Try
            If inArray Is Nothing Then
                FlagEx = False
            ElseIf inArray.Length <= 0 Then
                FlagEx = False
            ElseIf inArray(0) Is Nothing Then
                FlagEx = False
            End If
        Catch ex As Exception
            FlagEx = False
        End Try
        Return FlagEx
    End Function

#End Region
End Module

Answer 1

• Option Strict

  First thing first: Enable Option Strict please read : whats-an-option-strict-and-explicit

  This will make your code less error prone. If you have enabled Option Strict you will see a warning on the line ElseIf inArray(0) Is Nothing Then that late binding is not allowed if using Option Strict On.

---

• Regions

  Please read are-regions-an-antipattern-or-code-smell

  Is there a good use for regions?

  No. There was a legacy use: generated code. Still, code generation tools just have to use partial classes instead. If C# has regions support, it's mostly because this legacy use, and because now that too many people used regions in their code, it would be impossible to remove them without breaking existent codebases.

  Think about it as about goto. The fact that the language or the IDE supports a feature doesn't mean that it should be used daily. StyleCop SA1124 rule is clear: you should not use regions. Never.

---

• Hungarian notation

  The parameter name inArray is a sign, that you are misusing the original intention. Please read this: what-is-the-benefit-of-not-using-hungarian-notation

  And don't forget to read the reffered links.

---

That beeing said let us check how we could enhance this method by grouping the conditions together.

By using OrElse or AndAlsowe can short circuit conditions, which means that if a conditionA is true the conditionB won't be evaluated. like

If conditionA OrElse conditionB Then  

End If  

If we invert the conditions we can simplify the whole method to

Module Module2

    <Extension()>
    Public Function IsAllocated(ByVal inArray As System.Array) As Boolean

        Return Not ((inArray Is Nothing) OrElse (inArray.Length = 0) OrElse (inArray.GetValue(0) Is Nothing))

    End Function

End Module

Answer 2

It's not bad. An extension method is a nice choice here. There's no state and you're extending a class that you don't own/can't directly modify. Good design decision. I also like that you're using the Try...Catch paradigm. A common mistake for VB6 devs moving to .Net is carrying over the VB6 error handling pattern.

One thing I don't like about what you've done is this.

Catch ex As Exception

This catches every possible exception in the world, including potentially fatal memory errors. Be as specific as possible when catching exceptions. What specific exception would you expect to happen? Catch that instead, or none at all.

The only other thing I want to mention is that FlagEx is a poor name and the variable itself is kind of useless. Take advantage of the Return statement.

         Try
             If inArray Is Nothing Then
                 Return False
             ElseIf inArray.Length <= 0 Then
                 Return False
             ElseIf inArray(0) Is Nothing Then
                 Return False
             End If

         Catch ex As Exception
             Return False
         End Try

         Return True

Of course, you could take this a step further by doing away with the Elifs.

         Try
             If inArray Is Nothing Then Return False
             If inArray.Length <= 0 Then Return False
             If inArray(0) Is Nothing Then Return False

         Catch ex As Exception
             Return False
         End Try

         Return True

Which leads us to using some short circuiting.

         Try
             If inArray Is Nothing 
                 OrElse inArray.Length <= 0 
                 OrElse inArray(0) Is Nothing Then 

                 Return False
             End If

         Catch ex As Exception
             Return False
         End Try

         Return True

Then flip the condition so we can directly return.

         Try
             Return (inArray Is Not Nothing 
                 AndAlso inArray.Length > 0 
                 AndAlso inArray(0) Is Not Nothing)

         Catch ex As Exception
             Return False
         End Try