Making my Resource code better?

[groston]

In my VB.net (2012) program, I defined some resources. I did this by right-clicking on the project name in the Solution Explorer window and choosing properties. From there, I picked the Resources tab the Add Resources -> Add Existing File. I also have the following code:

In the class, but before any routines:

Structure imInfo
    Dim strName As String
    Dim brPtr As Bitmap
End Structure

Dim aryInfo(6) As imInfo

In the form’s Load routine:

aryIno(0).strName = "name1"
aryIno(0).brPtr = My.Resources.name1
aryIno(1).strName = "name2"
aryIno(1).brPtr = My.Resources.name2

In a button click routine:

For t = 0 To cntImages
    images(t).strFName = <entered string>
    images(t).bm = getBMfromName(images(t).strFName)
Next

And the routine called:

Private Function getBMfromName(ByVal resName As String) As Bitmap
    For i = 0 To 6
        If aryIno(i).strName = resName Then
            getBMfromName = aryIno(i).brPtr
            Exit Function
        End If
    Next
End Function

The code works just fine and the aryInfo array only has seven elements and is unlikely to change. However, the manner in which I did this strikes me as being both ugly and inefficient. Is there a better/cleaner way to accomplish this?

[Shaggy Hiker]

There are a few. One that you might consider is a Dictionary(of String, Bitmap). This would remove the need for that structure, as it directly ties the key (the name) with the image. Looking up an image by the key is considerably faster than iterating through an array, though with only seven items, "considerably faster" is pretty meaningless. Still, your GetBMFromName method would become:

Return yourDictionary(resName)

You could make that a bit more robust if you had a call to yourDictionary.ContainsKey to check whether or not the key is present, but that isn't necessary if you KNOW that you won't be calling the function with anything other than valid keys.

Another alternative is to not do that at all. After all, the resources are practically a dictionary anyways. I suppose it might be a bit more complicated to look up the right resource based on a name, and you can't beat the efficiency of a true dictionary lookup, but with only seven items, anything is fast enough.

[jmcilhinney]

You don't even need your structure in the first place. Resources already have names and you can get them by that. Instead of this:

Dim bmp = My.Resources.SomeBitmap

you can do this:

Dim bmp = DirectCast(My.Resources.ResourceManager.GetObject("SomeBitmap"), Bitmap)

Just use that in your getBMfromName method and you're done. For the record, that's exactly what the property does internally already.

Just note that getting data from My.Resources is going to create a new object every time, so it's not something that you want to do a lot in a short space of time. If you need to use these images repeatedly then the Dictionary may still be the best option.

BTW, getBMfromName is a terrible method name. That should be GetBitmapByName. Method names should start with an upper-case letter - you can use a different convention if you want but there's really no good reason to do so, given that the whole .NET framework is that way. Using abbreviations like BM to save 4 characters in this age of Intellisense is just silly, making code less readable for no gain. By makes more sense than From in this scenarion - the Bitmap is coming from resources, not from the name - and all words in the name should start with an upper-case letter.