Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Late binding #219

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Late binding #219

wants to merge 3 commits into from

Conversation

Deedolith
Copy link

Suggestion to switch the library to Late binding to prevent known issues with Early bound libraries (type names conflicts).

Switching Scripting.Dictionary to late binding.
@Nick-vanGemeren
Copy link

Surely this should be a compilation option? ParseJSONcan create manyDictionary.

@stenci
Copy link

stenci commented Aug 29, 2022

I don't agree with this PR, it has a horrible effect on performance. Late binding can be hundreds of times slower than early binding.

The solution to the naming conflict is not to introduce sloppy performance problems, it is to not use ambiguous names, that is use the library name prefix.

Here is a quick test that shows how bad late binding can be:

Sub Test()
  Dim T As Single, O As Object, I As Long, D As Dictionary
  
  T = Timer
  For I = 1 To 10000
    Set O = CreateObject("Scripting.Dictionary")
    O.Exists "key"
  Next I
  Debug.Print Format(Timer - T, "0.000")
  
  T = Timer
  For I = 1 To 10000
    Set D = New Dictionary
    D.Exists "key"
  Next I
  Debug.Print Format(Timer - T, "0.000")
End Sub

Output:

3.453
0.039

@Deedolith
Copy link
Author

Afraid this is a naive approach.
First, performances issues must be analysed with profiling tools, and they must be the least of your concerns during development steps (readability and producing the expected behavior are the top ones).

Second, name conflict are not always solved as easilly as you think (immagine the nightmare on project with million of lines of codes and more dependencies than fingers on your hands and feets), impacts can go from minimal (easy case) to hundred of days of work (nightmare case).

Finally, name conflict is only one of the issue provided by early binding, version conflict is probably the worst one (remember that developers have no controls on end user computers).

@stenci
Copy link

stenci commented Aug 29, 2022

I have been using this module for years. In the past only for small data structures because it was unusable on some computers because it was too slow. Converting the same dictionary to JSON on my computer and a few other ones would take 1 second, but there were 5-6 computers on my company where it would take 15 minutes.

After upgrading to one of the latest revisions (I don't remember which one, I think there were improvements on how the buffers are managed) all of a sudden it became fast on all computers and I have finally started using it also with large data structures.

Switching from early to late binding would force me to abandon this module again and go back to my personal tricks to solve performance problems.

The test in my previous post has only 10000 calls to one method. I often have data structures that require much more method calls, late binding would make this module unusably slow and I would need to go back to my old contraption (or keep my branch with early binding :) )


True, name conflicts are not always solved so easily, but declaring the namespace is most of the time the first thing to try and the most likely to succeed (at least in my experience).


True, readability is very important, and both As Dictionary and New Dictionary are definitely more readable than As Object and CreateObject("Scripting.Dictionary").


True, producing the expected behavior is very important, and I can assure you that hiding behind late binding is the last resource and should only be used to when different computers could have different and incompatible versions of the same component, and the add-in uses only methods that are compatible across all the versions. But a Dictionary from scrrun.dll is definitely reliable enough to be available and compatible across any respectable computer.

@Nick-vanGemeren
Copy link

I think that, as far as possible, any changes to the existing behaviour of modules should be driven by options, the existing behaviour remaining the default. In this case, to avoid a compiler error, we need to use conditional compilation.

I hope we can agree thatjson_ParseObjectcan return an Object rather than Dictionary.

I propose:

‘ Compilation option:
‘----
‘ DictionaryBinding: can specify early or late binding of Scripting.Dictionary.
 ‘	MS Word: import VBA-Dictionary or define non-zero DictionaryBinding
‘	Mac: import VBA-Dictionary (Scripting.Dictionary not available)
 ‘ 	Default (0): use Dictionary class (VBA-Dictionary or early-bound Scripting.Dictionary)
‘	To set, uncomment one of the following or define in the VBA project Properties (colon-separated list)
‘#Const DictionaryBinding = 1     ‘ Early bind Scripting.Dictionary 
‘#Const DictionaryBinding = -1    ‘ Late bind Scripting.Dictionary
‘----

and then injson_ParseObject:

#If Mac Then
    Set json_ParseObject = New Dictionary
#ElseIf DictionaryBinding = 1 Then
    Set json_ParseObject = New Scripting.Dictionary
#ElseIf DictionaryBinding = -1 Then
    Set json_ParseObject = CreateObject("Scripting.Dictionary")
#Else
    Set json_ParseObject = New Dictionary
#End If

This is backwards compatible and allows full flexibility for Windows users. No code modification is needed if the project properties are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants