Webelements.IsMember modifications

[MarkJohnstoneGitHub]

A suggestion for the Webelements.IsMember function

Public Function IsMember(element As WebElement) As Boolean
    Dim i As Long, elemId As String
    elemId = element.ElementId
    IsMember = False
    For i = 1 To Me.Count
        If Me.Item(i).ElementId = elemId Then
            IsMember = True
            Exit For
        End If
    Next i
End Function

It appears it could be more appropriately renamed to Exists .

The code appears inefficent and can simplified.
You only are required to pass in the string key i.e. element.id I'm not sure how you generated/acquire the initial ElementId key so far examining the code, not important atm, thou curious.

The below code should/not tested work.

''
'@Description Returns True if a specified key exists in the colWebElement VBA.Collection; False if it does not.
'@param key String key value being searched for in colWebElement VBA.Collection.
'@returns (Boolean) True if a specified key exists in the colWebElement VBA.Collection.
' ------------------------------------------------------------'
Private Function Exists(ByRef key As String) As Boolean
    On Error GoTo ErrorHandler
    colWebElement.Item key
    Exists = True
CleanExit:
    Exit Function
ErrorHandler:
    Exists = False
    Err.Clear                                    'reset error trapping.
    Resume CleanExit
End Function

I constructed it from the code below from my DictionaryKeyValuePair class. You don't require the EncodeKey related code as already have a String key and it converted various data types including objects to String keys.
https://github.com/MarkJohnstoneGitHub/VBA-IDictionary/blob/master/scr/DictionaryKeyValuePair.cls

''
'@Description("Returns True if a specified key exists in the dictionary; False if it does not.")
'@param Key The key value being searched for in the Dictionary object.
'@return {Boolean}
'@Error 5  Invalid procedure call or argument.
'          Raised for invalid/unsupported key data type from encoding the collection key.
' ------------------------------------------------------------'
Public Function Exists(ByRef Key As Variant) As Boolean
    Dim keyExists As Boolean
    keyExists = False
    Dim encodedKey As String
    encodedKey = EncodeKey(Key)
    If EncodedKeyExists(encodedKey) Then
        keyExists = True
    End If
    Exists = keyExists
End Function

''
'@Description Returns True if a specified key exists in the collection object; False if it does not.
'@param encodedKey Endcoded string key value being searched for in the collection object.
'@returns (Boolean) True if a specified key exists in the collection object.
' ------------------------------------------------------------'
Private Function EncodedKeyExists(ByRef encodedKey As String) As Boolean
    On Error GoTo ErrorHandler
    this.keyItemPairs.Item encodedKey
    EncodedKeyExists = True
CleanExit:
    Exit Function
ErrorHandler:
    EncodedKeyExists = False
    Err.Clear                                    'reset error trapping.
    Resume CleanExit
End Function

[MarkJohnstoneGitHub]

Having a quick explore at other classes you've got similar functionality for handling collections. Hinting at simple Collection wrapper for
string keys only, which is one of the reasons was suggesting the name change to Exists. Possible have done code that might take some digging around to find it as its been a few years since touched VBA. Probably quicker to do from scratch.

[6DiegoDiego9]

+1 for the renaming to Exists, for analogy with VBA dictionaries
-1 for passing the elem.ElementId (more complex for beginners and longer to write)

If we're going to accept the deprecated use of errors as a shortcut, I'd try this:

Public Function Exists(element as WebElement) As Boolean
    On Error GoTo err
    Exists = True
    IsObject(Me.item(element))
    Exit Function
err:
    Exists = False
End Function

Not sure about the performance gain of all both the suggested solutions though, they should be tested.

[MarkJohnstoneGitHub]

@6DiegoDiego9
"+1 for the renaming to Exists, for analogy with VBA dictionaries"
That was the first thing that came to mind the naming consistancy from Scripting.Dictionary etc.

-1 for passing the elem.ElementId (more complex for beginners and longer to write)
Public Function Exists(ByRef key As WebElement) As Boolean

Ok it's slightly clearer that the key ElementId was probably obtained by Str(ObjPtr(objWebElement)) when added. I hadn't tracked down when added to the collection the source of key.

IsObject(Me.item(element))
Argh don't you require a string key for the VBA.Collection?
If the key elementId is acquired from its object pointer the below code should work.

Examing DictionaryKeyValuePair.EncodeKey, Private Function EncodeKey(ByRef key As Variant) As String
For encoding objects as a key: Str(ObjPtr(key))

Public Function Exists(ByRef key As WebElement) As Boolean
On Error GoTo ErrorHandler
colWebElement.Item Str(ObjPtr(key))
Exists = True
CleanExit:
Exit Function
ErrorHandler:
Exists = False
Err.Clear 'reset error trapping.
Resume CleanExit
End Function

Imho cleaner to have one exit point. Hmm either way ok yours reflects the Try Catch pattern better IMO.
From memory require to reset the error trapping.

Argh from a quick refresher on error handling. https://excelmacromastery.com/vba-error-handling/
"Err.Clear is used to clear the text and numbers from the Err.Object. In other words, it clears the description and number.If you want the clear the actual error you can use either On Error GoTo -1 or Resume Next"

I might have to looking that cant recall where got the recommendation Err.Clear

Also see the same code in WebCookies, strongly hinting a Collection wrapper extended with Exists might be useful.

Require a refesher on VBA.Collection after not touching VBA for a few years.

[GCuser99]

@MarkJohnstoneGitHub, did you mean to declare your Exists function as Private?

Like @6DiegoDiego9, I think the name change is an improvement and also agree that "trafficking" in ElementId's breaks a design choice. It was decided early on in development that ElementId's would be hidden from the public-facing object model. I'm good with either of the proposed solutions, but Mark's will have to be modified to something like:

Public Function Exists(element as WebElement) As Boolean
    Dim key as string
    key=element.ElementId 'friend property
...

[MarkJohnstoneGitHub]

@GCuser99 Argh, nice catch. Opps caught me copying and modifying the code from DictionaryKeyValuePair.EncodedKeyExists
Will edit that. On second thoughts if you can edit to Public so don't lose the code formatting block. Thanks.
Question. How to keep the code formatting?

[MarkJohnstoneGitHub]

@GCuser99 Comes down unsure about the element.ElementId key for the collection. Any pointers where to find its source when addded to the collection to help better understand? Hmm the key is hidden within the WebElement object. Maybe/preferable could use the object pointer as the key converting it to a string.
Str(ObjPtr(element))

[MarkJohnstoneGitHub]

@GCuser99
Public Function Exists(element as WebElement) As Boolean
Dim key as string
key=element.ElementId 'friend property
...

That might work with using a Collection wrapper with the Exists function. In the Implements section would refer to the Public Exists(element as WebElement) . As duplicating code in WebCookies a VBA.Collection wrapper would remove duplicated code. If no need for lists of keys or items should be reasonable easy to implement. Downside would have to change code to use the Collection wrapper interface in Webelements and WebCookies.

[GCuser99]

@MarkJohnstoneGitHub - how to keep code formatting? I enclose code block using three of the accent char (on same key as the ~, in the upper-left corner of my keyboard) followed by "vba" (without a space between the triple accents and vba) to open the block and then triple accents to close.

I think a collection wrapper would be over-kill for this. Let's change the IsMember method to Exists. I'm not in favor of changing the input parameter "element as WebElement" to "key as WebElement" as the user shouldn't need to be aware how Exists works (collections and keys) but only that it DOES work.

So presumably the coding problem we are trying to solve here (aside from the rename) is one of style (with only several thousand lines of code left to scrutinize! :-), and maybe efficiency... I'm ok with any of the Exists variants above, unless efficiency difference turns out to be significant (I doubt it). I like @6DiegoDiego9's version because it is succinct, and I can understand it at a glance. I also like your first iteration but with the modifications I suggested regarding the change to input parameter spec. PS: I like my original version too, but it's likely less efficient - I'd be curious to see how much less efficient!

Can you test your proposed method against @6DiegoDiego9's, and my original for efficiency?

Thanks for your suggestions and insights.

BTW, are you able to run SeleniumVBA tests in MS Access yet?

[MarkJohnstoneGitHub]

@GCuser99 Will look at performance when get some time could be a few days. Offhand its VBA.Collection hashtable verses on average iterating half of the size of the collection. Consideration depending on the size of collection you're expecting as the larger it gets the more it effects performance.

A VBA.Collection wrapper might be useful in other projects to help simplify code.

[6DiegoDiego9]

Anyway, the combinated need of Key handles and Exists method is usually a sufficient reason for me to use a Dictionary instead than a Collection.
@GCuser99 is there a good reason for keeping it a Collection?
Maybe Mac compatibility?

[MarkJohnstoneGitHub]

@6DiegoDiego9 What was going through my mind. Apart from being Mac compatibile then need a collection based dictionary. Tempted to rename my DictionaryKeyValuePair to CollectionDictionary.

Hmm Mac compatiblity you've possible got some Mac incompatible code, so really trying to keep it Mac compatible? If designed well making Mac compatible could be feasible.

[GCuser99]

@6DiegoDiego9 - no reason for it NOT being a dictionary. Although again, I'm having difficulty seeing what we are trying to solve here. Does a Collection fail in some way? Or is the Exists code so ugly and confusing that it needs a Dictionary (or a new Collection wrapper) to make it significantly easier to maintain? Or is there a significant efficiency problem?

There are thousands of lines of code in this project, which I will be the first to admit is an amalgam of different and inconsistent styles (the original author of TinySeleniumVBA, an anonymous poster on that site who wrote a big chunk that was not incorporated into that project, myself, and you). The impetus when I got involved was to achieve near complete Selenium functionality as quickly as possible so I and others could use it! I think we are now close to achieving that goal (thanks again for your key contributions towards that!). Therefore, it makes sense next to try to standardize those contributions as far as style and maintainability - that's one of several reasons why I asked you if you were interested in helping. That is going to be a significant amount of work and I don't expect it to happen fast. I'm excited about tools like RubberDuck (thanks @MarkJohnstoneGitHub!) that will help us with that.

However, our time is valuable, and we need to be selective on what we work on. I'm not in favor of making changes without compelling justifiable reasons. Finding a way to articulate justification for change, as a community, is what these discussions are very useful for - so I really appreciate the back and forth like above.

As stewards we also need to be willing to balance our own (or any one individual user's) needs against the (potential) community of user's needs, with more weight on the later, IMO. I have a handful of ideas that I would like to add to SeleniumVBA to make it more useful to me, but I resist the urge to implement because if I'm honest, implementing some of these would probably not be very useful to most of that hypothetical community.

Anyway, I apologize for what may seem like a rant - I'm just trying to explain above where I'm coming from when I may seem resistant or critical, when all I am sincerely trying to do is be a responsible steward - really! :-)

[6DiegoDiego9]

@GCuser99 I'm on your line, we're talking about possible gains of ~1μs (10⁻⁶) per element in a collection, in a context of running times of 1+ seconds per each page change. My average collection is about as big as 5 elements and even if we worked with collections of hundreds of elements, the gain would still be not significant to me.

I also am testing some new features, addable to this project, struggling to find time, among other projects in my life, to properly clean/complete and test them:

• tiny status bar (eg. during driver downloading, during browser opening, during multiseconds page loading times)
• true parallel/multithreading/async processing of entire procedures (VBscript method)
• regex search (in text of elements of a collection or of whole page)
• continuable timeout (msgbox with "Continue waiting"/"Terminate" button)
• others...

I think that you're a perfect responsible steward, I'm really enjoying collaborating with you :-)
I just would like to free up and find time to do much more!

[MarkJohnstoneGitHub]

@6DiegoDiego9 Depends on size, frequency of use, etc for what what appropriate data structure to use. Arrays verses colllection/dictionary/link lists etc. For what was looking at using it for could have much larger datasets. So performance might become an issue. It's "easier" to avoid any potiential issues.

Mutithreading advance topics briefly looked into that. Mathieu Guindon "VBA Guru" (Rubber Duck) might be able to help if not busy.