Using a nonsense string to check if a call has completed, rather than timeout?

Question

My question is whether or not this is a good practice, and/or if there is a more efficient way to do it.

I have to access this Solidworks interop object and wait for it to return some value. The return is a string. In early testing I found I ran into issues if I didn't wait for the return, so I added a "DoEvents" wait loop using While len(strProp)=0 to wait for the string to populate so I would know the call had executed and returned something.

The issue I then ran into was that when there is no value in the interop object's property, the return IS a zero-length string. So it would just loop forever, even though the call had completed.

Normally I would just use a "time-out" value - e.g. get the current time, and have an exit loop statement when a certain amount of time has passed. However, in testing I found sometimes the call could actually take over a minute to complete. Since I'm processing thousands of properties at a time, I thought I would be wasting a TON of time waiting for empty ones. So I came up with the idea of setting the string to a "nonsense" value before the call to the interop object, and then checking for when it changes to know that the call has completed.

Private Function GetModelProps(ByRef swCustPropMgr As SldWorks.CustomPropertyManager, Optional ByVal dicInput As Scripting.Dictionary) As Scripting.Dictionary
'This procedure gets the properties from a custom property manager object that is passed by the calling procedure. It returns a dictionary object containing the properties found.
'Declare constants
 Const strNonsense As String = "SKDjf09sf!@&#(@#$&!(*^)_!@0-12-849uj0394jfIODJS(G_3r5902-129381203892193018" 'This is just a nonsense string that will never be entered in a custom property
'Declare variables
 Dim lngX As Long
 Dim strProps() As String
 Dim strProp As String
 Dim strPropStatement As String
 Dim strPropVal As String
 Dim dicProps As Scripting.Dictionary
 On Error GoTo CleanExit
'Initialize dictionary object
 Set dicProps = New Scripting.Dictionary
 'Pass values from the input dictionary if one was provided
 If Not dicInput Is Nothing Then
 If Not IsArrayEmpty(dicInput.Keys) Then
 For lngX = LBound(dicInput.Keys) To UBound(dicInput.Keys)
 DicMergeOnKey dicProps, dicInput, (dicInput.Keys(lngX))
 Next
 End If
 End If
'Attempt to get the names of all the custom properties. If there are no custom properties the array will not be initialized and populated.
 On Error Resume Next
 strProps = swCustPropMgr.GetNames
 On Error GoTo CleanExit
 'Check to see if the array is empty - if it is, then there are no custom properties to get
 If Not IsArrayEmpty(strProps) Then
 'Loop through all the custom properties
 For lngX = 0 To UBound(strProps)
 'Get the custom property name
 strProp = strProps(lngX)
 'Set initial value to a string that will never be found
 strPropVal = strNonsense
 'The Get2 method takes one input and returns 2 strings: Input: Property name, Output1: Property statment, Output2: Evalutated property value
 swCustPropMgr.Get2 strProp, strPropStatement, strPropVal
 'Sometimes this can take a while to execute, so we use the loop below to wait until the system is done processing all other events
 While strPropVal = strNonsense
 DoEvents
 Wend
 'Make the property uppercase so we don't accidentally add multiple properties due to difference in case between documents
 strProp = UCase(strProp)
 'Verify item numbers before adding
 If strProp Like "PARTNO" Then
 strPropVal = ValidateLokItem(Trim(strPropVal))
 End If
 'Add the returned value to the dictionary.
 If Len(strPropVal) > 0 Then
 If Not dicProps.Exists(strProp) Then
 dicProps.Add strProp, strPropVal
 Else
 dicProps.ITEM(strProp) = strPropVal
 End If
 End If
 Next
 End If
'Return the dictionary to the calling procedure
 Set GetModelProps = dicProps
CleanExit:
 Set dicProps = Nothing
 Set dicInput = Nothing
 Erase strProps
End Function

• RubberDuck: I encourage you to give it some time between questions. What you learn in one review will likely be applicable to other code.
• CBRF23: Thanks RubberDuck. First time on CodeReview. Wasn't expecting a review of the whole code, just the part in question. I edited my code sample to reflect.
• RubberDuck: Your last edit made this stub code IMO. I rolled it back.

Answer 1

I like that you're being explicit about how you're passing arguments.

Private Function GetModelProps(ByRef swCustPropMgr As SldWorks.CustomPropertyManager, Optional ByVal dicInput As Scripting.Dictionary) As Scripting.Dictionary

But I can't find an instance where swCustPropMgr is being assigned to. As such, you can pass it ByVal too and eliminate an entire class of possible future bugs from the code.

A lot of times, we can replace good comments with well named procedures. I think this is one of those cases.

'Initialize dictionary object
Set dicProps = New Scripting.Dictionary
'Pass values from the input dictionary if one was provided
 If Not dicInput Is Nothing Then
 If Not IsArrayEmpty(dicInput.Keys) Then
 For lngX = LBound(dicInput.Keys) To UBound(dicInput.Keys)
 DicMergeOnKey dicProps, dicInput, (dicInput.Keys(lngX))
 Next
 End If
 End If

I'd extract the whole thing into a private function.

Set properties = InitializePropertDict(inputDict)

There are other comments like this. This one is notable.

'Attempt to get the names of all the custom properties. If there are no custom properties the array will not be initialized and populated.

In other words, TryGetProperties.

In early testing I found I ran into issues if I didn't wait for the return, so I added a "DoEvents" wait loop using While len(strProp)=0 to wait for the string to populate so I would know the call had executed and returned something.

I would look deeper into this problem, because this feels hacky. (Mostly because it is a hack to make it work.)

swCustPropMgr.Get2 strProp, strPropStatement, strPropVal
 'Sometimes this can take a while to execute, so we use the loop below to wait until the system is done processing all other events
 While strPropVal = strNonsense
 DoEvents
 Wend

I honestly don't understand why this works at all. It shouldn't. VBA is single threaded so strPropVal should either be set or not by time you reach the While loop. The call to .Get2 should block the execution from moving forward until it's completed. If it's not, consult the docs and see if that function returns any values. If it does, you might be able to force it to block by "using" that return value to set a variable.

junk = swCustPropMgr.Get2(strProp, strPropStatement, strPropVal)

Either way, the comment needs to be much more thorough and clear.

While we're here, I have a convention for variable names that are used to return values from out params like this. I think prefixing them with out makes the intention much clearer. I've been bitten by this before..

swCustPropMgr.Get2 strProp, strPropStatement, outPropVal

Some would call this Hungarian notation done right.

---

I actually did just look up the docs and it seems that Get2 does return void (in VBA terms, it's a Sub), so my advice isn't very practical. However, it's also been marked as Obsolete and the newest version of that method, Get4 returns a boolean. So, I'd give that a shot. They're also saying that Get4 uses caching to speed up the call, so you could see a significant performance boost as well.

• CBRF23: Splitting out some of the statements into a function is something I should probably be doing more. SW changes their functions with every release, I'll look into Get4! But, I don't understand why DoEvents is "hacky". Can you recommend a non-hacky method to replace? What I found in testing was I a problem where occasionally properties weren't being captured. This was tough to troubleshoot because sometimes .Get2 executes immediately - but other times it takes upwards of 2 minutes to return. VBA does not wait for this return, which is why I added a DoEvents loop to make it wait.
• RubberDuck: Give my answer another careful read. I explained most of that already. As for the DoEvents, it simply frees up the OS to do other things and can cause other issues. So, it is a "hack" to make VBA "wait" on the return. Which, like I said, shouldn't be happening anyway. I explained a method you can try to solve this in a cleaner way.
• Mathieu Guindon: @CBRF23 VBA runs on a single thread, it cannot possibly be awaiting (as in async/await) an API call.
• Mathieu Guindon: OTOH it could be that the API itself runs async, and yields a reference to the unpopulated result. That would explain the behavior, but not why it doesn't return immediately systematically. I'd look for a way to get the results synchronously.
• CBRF23: @RubberDuck - I plan to upgrade to the newer Get4 function, but I would still like an example of how you would handle this without using DoEvents. At the time I wrote the code, Get2 was the latest procedure available - I'd like to know how else I could have handled the wait? @Mat'sMug - I would imagine something like you describe was happening. I don't have any way to tell. All I know is that I was able observe the behavior I described first hand of VBA executing the next line before getting the return. This would happen seemingly randomly, maybe 5% of the total executions.