-
Notifications
You must be signed in to change notification settings - Fork 103
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
Fixed compilation warning related to adding and removing of event handler in ucrSave #6480
Fixed compilation warning related to adding and removing of event handler in ucrSave #6480
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, if you can correct the corrupted indents, then I can approve
instat/ucrSave.vb
Outdated
End If | ||
End If | ||
UpdateAssignTo() | ||
End Sub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indents somehow got corrupted, please could you correct? thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lloyddewit below is a screenshot of why the indents are to the far right. This is because the subroutine is assigned to the variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Patowhiz thank you, now I see what's happening.
I think there is a simpler solution. I think the warning is generated because LinkedReceiverControlValueChanged
does not have the signature that VB expects for a handler.
If we declare the sub as Private Sub LinkedReceiverControlValueChanged(ucrChangedControl As ucrCore)
, then it should fix the warning:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lloyddewit I tried this after seeing the same on Stack Overflow but this will make
AddHandler Me.ucrLinkedReceiver.ControlValueChanged, LinkedReceiverControlValueChanged
have a syntax error of "the expression does not produce a value", which will force me to set the delegate as a function that always returns true making the remove handler "complain" again because of the anonymous object it will always create when executed.
These are all misdoings of VB.NET lax features, it tries to cast everything for you when it thinks it can, even when it shouldn't its why I missed it in the first PR.
Could be I missed something in their documentation, please could you try it on your end cause if it works then this will phenomenally simplify the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lloyddewit after our zoom discussions, my findings(after doing the tests and reading the documentation) show that your proposition also fixes the issue. Since its much simpler I have done the code changes.
Briefly, just to explain my conclusions from my findings , VB.NET creates the sub event handler as an anonymous instance delegate and keeps it in the memory. Every time a call is made to the subroutine in a manner that requires the subroutine to be used as an instance delegate, it just uses the one in the memory instead of creating a new one.
So these shared delegate instances are more or less managed like the way we have shared instances of dialog forms in R-Instant, that is, we do;
dlgFrequency.ShowDialog()
instead of
Dim dlgFrequency As New dlgFrequency
dlgFrequency.ShowDialog()
and we are able to just access dlgFrequency
without explicit initialization in other classes and the instance will still be the same.
This is phenomenally different from other OOP languages and some of these lax features I think are due to historical reasons(since VB.NET succeeded VB6).
My understanding is, this works fine because the compiler is designed to created those shared instances for us. Same happens for delegate instances created from functions and subroutines in a class, when compiling the code.
Below are links to some of the Microsoft official documentations that I combed through;
AddressOf Operator.
Delegates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Patowhiz Thanks for the investigation and testing.
@rdstern @shadrackkibet I don't think that there's anything that can be tested here but if one of you can approve, then we can merge, thanks |
Fixes #6509 issue raised in comment of PR #6434.
@shadrackkibet @lloyddewit this is ready for your review.