I am using Func(Of Boolean) as a conditional, but it doesn't seem to be working correctly. Even though the function returns False, when I use If Condition() Then... it still passes. I am now sure what is going on. I created a gif of the problem for those that want to see it in action. I had to put it in a zip because it kept uploading as a jpg.
Please just post the code next time. An animated gif in a zip file is one of the more difficult ways to share code, most people won't go through the multi-step process to view it. I can't even pause it to study the code! It took me at least 10x longer to figure out what was going on, and since I can't see all the methods I can only speculate. Here's what I think:
For the function to return false, then BFF.Main.Game must be nonzero. Nothing in your GIF proves this is the case. Have you verified that the value is indeed nonzero? How?
There's a slight chance you're confused about closures. Are you aware cond will always use the current value of BFF.Main.Game, and not the value from the time it was created?
I don't see where PerformReadAction() is called. Are you sure that cond represents the function it is being given?
My guess is one of those things is not operating the way you think. Give it a shot, and if it's still failing, change cond to always return False. Does it still appear to always return true? Now you know for sure the function that PerformReadAction() executes is not cond.
Here's a test application to run to prove to yourself that this indeed works:
Code:
Module Module1
Sub Main()
Dim value = 0
Dim conditionalFunction = Function()
Return value = 0
End Function
value = 10
Console.WriteLine(TellMeWhatItReturns(conditionalFunction))
End Sub
Function TellMeWhatItReturns(ByVal toExecute As Func(Of Boolean)) As Boolean
Return toExecute()
End Function
End Module
Last edited by Sitten Spynne; Aug 24th, 2015 at 09:47 AM.
This answer is wrong. You should be using TableAdapter and Dictionaries instead.
When I step through and get to PerformReadAction, the autos window shows the following:
Code:
‡ BFF.FF1.Character.<closure>.<lambda0-0> returned False Boolean
That shows that it is truly returning False once I get to the PerformReadAction method.
About your points:
1) BFF.Main.Game is equal to 1, which is why it chooses Return False in the gif.
2) The reason I use them is because of their dynamic nature; I want to use it as a conditional. PerformReadAction is called in a thread, and it checks the value of the Condition parameter each time. It needs to be dynamic.
3) I apologize. I thought it was in the gif. Below is the code where it is called.
vb.net Code:
Sub SetReadAction(Action As Action(Of ListBox), Optional Condition As Func(Of Boolean) = Nothing)
If Action Is Nothing Then
Common.GetError(New ArgumentNullException("Action cannot be Nothing."))
Exit Sub
End If
ReadAction = Action
If ReadActionThread Is Nothing OrElse Not ReadActionThread.IsAlive Then
ReadActionThread = New Thread(Sub()
While Not DesignMode
PerformReadAction(Condition)
Thread.CurrentThread.Join(25) ' Rest the thread to prevent lag.
End While
End Sub)
ReadActionThread.IsBackground = True
ReadActionThread.Start()
End If
' Process paint messages.
If CBool(NativeMethods.GetQueueStatus(&H20)) Then
Application.DoEvents()
End If
End Sub
I did change the code to always return false and it still passes.
You mentioned a thread. This thread wouldn't happen to be trying to touch the UI, would it? That generally leads to weird behaviors.
Also I'm still a little concerned by the Optional parameters. And I can't remember what the code in the gif said anymore, and this laptop doesn't run it well. The Autos window tells me what was going on at some moment in time, but have you tried putting breakpoints at relevant spots to watch it at the precise moments where it's called? Something's fishy, and it has the particular smell of "the function you are calling is not the function you expect". Since we're dealing with multiple layers of call stack, and threads, and closures, I can think of way too many potential problems to list.
It'd be nice to see the entirety of the code files, so we could piece it all together.
This answer is wrong. You should be using TableAdapter and Dictionaries instead.
No, the thread does not touch the UI. The solution is thousands of lines of code. I'll try to provide the step by step.
The initial call looks like this:
vb Code:
lbChar.ReadCustomText(Stats.Name, 4, cond)
ReadCustomText definition looks like this:
vb Code:
Public Sub ReadCustomText(data As CustomText, count As Int32, Optional condition As Func(Of Boolean) = Nothing)
SetReadAction(Sub(obj)
For i = 0 To count - 1
If lb.Items.Count <= i Then
lb.Items.Add(data(i).Value)
Else
If Not lb.Items(i).ToString = data(i).Value Then
lb.Items(i) = data(i).Value
End If
End If
Next
If lb.SelectedIndex = -1 AndAlso lb.Items.Count > 0 Then
lb.SelectedIndex = 0
End If
While lb.Items.Count > count
lb.Items.RemoveAt(lb.Items.Count - 1)
End While
End Sub, condition)
End Sub
As you can see, condition is being passed through until it reaches the relevant function.
SetReadAction look like this:
vb Code:
Sub SetReadAction(Action As Action(Of ListBox), Optional Condition As Func(Of Boolean) = Nothing)
If Action Is Nothing Then
Common.GetError(New ArgumentNullException("Action cannot be Nothing."))
Exit Sub
End If
ReadAction = Action
If ReadActionThread Is Nothing OrElse Not ReadActionThread.IsAlive Then
ReadActionThread = New Thread(Sub()
While Not DesignMode
PerformReadAction(Condition)
Thread.CurrentThread.Join(25) ' Rest the thread to prevent lag.
End While
End Sub)
ReadActionThread.IsBackground = True
ReadActionThread.Start()
End If
' Process paint messages.
If CBool(NativeMethods.GetQueueStatus(&H20)) Then
Application.DoEvents()
End If
End Sub
It's a thread that continuously calls PerformReadAction. The point of Condition is to only read when it returns True. This way, I can control the thread easier.
PerformReadAction looks like this:
vb Code:
Sub PerformReadAction(Optional Condition As Func(Of Boolean) = Nothing)
Dim sw As Stopwatch = Stopwatch.StartNew
If Condition Is Nothing OrElse Condition() Then
ExecuteSecure(Sub() ReadAction(Me))
End If
If Not OnReadAction Is Nothing Then
If OnReadCondition Is Nothing OrElse OnReadCondition() Then
ExecuteSecure(Sub() OnReadAction(Me))
End If
End If
sw.Stop()
End Sub
From here, it invokes ReadAction. The definition of ReadAction is:
vb Code:
Private ReadAction As Action(Of ListBox)
As you can see, Condition is simply passed through each method until it reaches PerformReadAction, where it is invoked. I said in my last post that Condition is False when it reaches PerformReadAction. Also, I only use Condition one time in this entire project. There are no other times it is used.
Lambdas aren't broken, so somewhere along the chain there's a line of code that is doing something that's not quite what you expect. To find that, I find it helps to get someone to tell me what *they* think each line does. When we disagree, I find my mistake. You can have this conversation with the debugger, have you tried this?
I'm assuming we're looking at this unit, because everything else seems simple.
Code:
If Condition Is Nothing OrElse Condition() Then
ExecuteSecure(Sub() ReadAction(Me))
End If
I don't know what you expect, because all I know is that you say something here is "wrong". I think this means you expect a False condition to NOT call ExecuteSecure(), but it IS calling ExecuteSecure. That's because if I pick this code apart, it says:
"If there is no condition, or if the condition is true, then call ExecuteSecure() with a lambda that calls ReadAction()."
We know the left half has to be False, because you've provided a call stack where Condition cannot be Nothing. So the only thing left is that Condition() must be returning True. This would be weird, since you hardcoded it to False. So let's do one last sanity check. Rewrite your code this way:
Code:
Dim noCondition As Boolean = Condition Is Nothing
Dim conditionResult As Boolean = True
If Not noCondition Then
conditionResult = Condition()
End If
If noCondition OrElse conditionResult Then ' Put a breakpoint here.
ExecuteSecure(Sub() ReadAction(Me))
End If
Put a breakpoint where indicated. Run your code until you hit the breakpoint. Look at the value of both variables. Are they "False" and "False"? Then all is working as expected and you need to explain what you think this code does so we can rewrite it to do that. Are they "False" and "True"? Then there is no other explanation than somehow cond is getting reassigned to a different lambda. That's difficult to unwind, especially when I have to ask for bits of code to be revealed.
This is still a little iffy since we are on a thread, but if you've indeed hardcoded cond to return false we should've isolated that.
So, to move forward:
I can't see what ExecuteSecure() does, but as of right now you are wrong when you say "the thread does not touch the UI". The thread calls a method that executes a lambda that updates a ListBox. That is most certainly touching the UI. Maybe ExecuteSecure() marshalls the call. I haven't seen proof, and strange things can happen in code when you do this.
You'd be surprised how fast I can dig through thousands of lines of code when I only care about 200 of them.
What are we actually doing? This seems like a complex architecture for anything, there might be an easier way to do this. Regardless, since all I know is "the if statement is acting up" it's hard for me to think about what could be going on in the larger context to cause it.
That third point is starting to dig at me.
It feels like this is some kind of editor for something in a game. It looks like the "ReadAction" thread is some kind of event loop: when buttons are clicked, you set its lambda. When it next ticks, it will evaluate a given condition and, if that condition is true, execute the lambda. Intuition tells me the condition lambda is there to make sure you don't continuously execute the ReadAction lambda.
That seems a little overengineered to me. It would be much more straightforward if you just do the thing when you know you need to do it. This is why I kind of want to step back and evaluate the architecture. I lack the context to decide if this looks like a good idea.
This answer is wrong. You should be using TableAdapter and Dictionaries instead.
Ok, you earned my respect. Your code snippet worked, but why? Is it because it gets invoked before the If statement? So the value is set beforehand which makes it work?
Also, it does seem over-engineered when looking at it like this, by itself. In fact, there are multiple controls that all use the same functionality. It's shared. You are correct that it's an editor for a game. Basically, I am reading values with the thread in a constant manner, so it's always up to date. Also, all these functions are in a separate class library that is shared by all my editors. This way, when I want to change some functionality, I only change it once instead having to update all the editors.
If you still want to evaluate the architecture, I can message you a link to my source files. It would be nice to have someone other than myself look at this stuff.
Edit: Here is ExecuteSecure()
vb Code:
Sub ExecuteSecure(a As Action)
If InvokeRequired Then
BeginInvoke(a)
Else
a()
End If
End Sub
Last edited by Troy Lundin; Aug 25th, 2015 at 02:33 PM.
Reason: Added code.
Hmm. That's kind of interesting. So you used the debugger and confirmed that the variables were "False" and "False"? I'm starting to think there's something subtle going on here. I tried a sample project but couldn't confirm any of the hypotheses I created.
If you were capturing cond as part of a closure, I'd blame that. But you're not. I wrote both versions of the code and on my machine they both produce "false" and "false" and don't call ExecuteSecure(). As far as I can tell, they're identical and shouldn't produce different results.
I keep wanting to blame the thread, but if you're hard-coding to False and not capturing cond in a closure you shouldn't be doing the things I worry are happening. It's a mystery. With the code before me, I can't answer it.
ExecuteSecure() looks mostly like what I'd expect, but I suggest Invoke() instead of BeginInvoke(). Usually you pair Begin/End calls together. The documentation here seems to imply it's not required, but there's also no need to use BeginInvoke().
I'm more interested in the source files to try and solve this dang puzzle than an architectural evaluation, if that makes any sense. I can't make a /promise/ I can look over everything, it'd definitely be the weekend before I could spend a lot of time on a review. But it sure would be nice to have a reason why this bit of code seems to misbehave.
This answer is wrong. You should be using TableAdapter and Dictionaries instead.