omegastripes / VBA-JSON-parser

Backus-Naur Form JSON Parser based on RegEx for VBA
GNU General Public License v3.0
108 stars 44 forks source link

selectElement() bug with certain paths #21

Closed TildenWinston closed 3 years ago

TildenWinston commented 3 years ago

Fully realize that selectElement() is in beta, but figured I should still mention this strange behavior.

So when I call selectElement() using the following it crashes silently.

Dim durationRawValVal
Dim durationRawValExists
Dim tempi As Integer
tempi = 0
Dim elementpicker As String
elementpicker = ".rows[0].elements[" & tempi & "].duration.value"
jsonExt.selectElement vJSON, elementpicker, durationRawValVal, durationRawValExists

Tracing the crash with the debugger points to line 740 in jsonExt. I wonder if it is an issue with reassigning the parameter "path".

I was able to work around the issue by adding a new variable to assign parts to. Not sure if this causes other errors or if it is the cleanest solution.

Public Sub selectElement(root, path, entry, exists)
 ...

    Dim elt
    Dim i
    Dim pathArray
    If Not IsArray(path) Then
        Dim parts
        If path = "" Then
...
        End If
        If Not exists Then Exit Sub

        pathArray = parts
    Else
        pathArray = path        
    End If
    assign root, entry
    exists = True
    For i = 0 To UBound(pathArray)
        exists = False
        elt = pathArray(i)
        If elt = "" Then
            exists = True
...

End Sub
omegastripes commented 3 years ago

Hi @TildenWinston, Thanks for the input.

Actually path string value is reassigned within selectElement() and replaced with array of path components. This is done with the aim of further reuse of the returned ByRefarray of path components in multiple selectElement() calls with the same path (e. g. in a loop) to improve performance. Once the path string splitting to components operation is completed, there is no need to do that again in the next selectElement() call, until you assign new value.

To fix the issue just declare the variable as variant type:

Dim elementpicker

Probably should be noted this feature somewhere in the manual...

TildenWinston commented 3 years ago

Ah, parameters are passed in a completely different way than I had expected.

Thanks for the help and clarification! That is a really neat way to handle it from a performance perspective!

omegastripes commented 3 years ago

Following the idea, I would retrieve .rows[0].elements array first, then iterate over the elements of the array in a loop (either For Each elt In arr or referencing by index For i = 0 To UBound(arr): Set elt = arr(i), and in this case I use the path .duration.value which is the same for each element (of course it's necessary to assign the path to variable prior to the loop, and pass the variable to procedure, but not the string constant directly). Note, this makes sense in a performance context for a large number of elements only.