views:

51

answers:

2

This works, and I can't imagine how it might cause problems, but visual studio gives me an warning and that makes me sad. I'm just wondering if doing something like this might ever cause problems:

I have a custom timer that acts like a Wait for some number of milliseconds and then execute a function. It looks like this:

Public Class MyTimer
    Inherits Timers.Timer

    Public Event Done()

    Public Sub New(ByVal interval As Double, ByVal repeat As Boolean, ByVal Work As DoneEventHandler)
        Me.AutoReset = Not repeat
    End Sub

    Private Sub ElapsedToDoneConvert() Handles Me.Elapsed
        RaiseEvent Done()
    End Sub
End Class

I use it like this:

Dim Timer as New MyTimer(1000, False, Sub()
                                        ..code..
                                      End Sub)

or

Dim Timer as New MyTimer(1000, True, Sub()
                                        ..code..
                                      End Sub)

The first case waits one seconds and then executes ..code.., the second case executes ..code.. repeatedly every one second.

Now the question: Imagine I have a form with a textbox called TextBox1 on it. Is this safe?

Dim Timer As MyTimer
Timer = New MyTimer(1000, True, Sub()
                                   If TextBox1.Text <> String.Empty Then
                                      MsgBox("TextBox1 is no longer empty")
                                      Timer.Stop()
                                   End If
                                End Sub)

(So, every one second, Timer checks if TextBox1 is empty. If it isn't, it displays a message box and stops checking.)

I get a warning that Timer is used before it has been assigned a value, but it is used in the statement that assigns its value. The timer's interval is required to be greater than zero. Is there anything about this that I don't understand that could cause problems?

Thanks for the help!

+1  A: 

You really should put that in a using statement, that way the timer is disposed of each time. Your MyTimer class should also implement IDisposable.

Installer
Hey, thanks for the comment. Does the fact that MyTimer inherits System.Timers.Timer do that for me?
BASnappl
You still need to explicitly call .Dispose, however, you probably don't need to implement IDisposable unless MyTimer itself has disposable elements.
Installer
+2  A: 

The problem is that you're using Timer in a lambda that you pass to the MyTimer constructor. When it compiles this line:

Timer = New MyTimer(1000, True, Sub()
                                   If TextBox1.Text <> String.Empty Then
                                      MsgBox("TextBox1 is no longer empty")
                                      Timer.Stop()
                                   End If
                                End Sub)

The Timer instance that you pass in could be used by the MyTimer constructor (the compiler doesn't know). If that's the case, and because the constructor runs before the result is assigned to Timer, you're passing an uninitialized value and you get the warning.

You can fix it quite easily:

Dim Timer As MyTimer = Nothing
Timer = New MyTimer(1000, True, Sub()
                                   If TextBox1.Text <> String.Empty Then
                                      MsgBox("TextBox1 is no longer empty")
                                      Timer.Stop()
                                   End If
                                End Sub)

That is, explicitly set it to "nothing" first. I think this would work, but in reality, even this is raising alarm bells to me. I would modify the API so that instead of requiring that you pass in an instance of the timer to the callback, you simply change it so that your callback returns either true or false for whether it wants to continue or not. That way, MyTimer itself can be responsible for stopping when the timer returns false.

Dean Harding
Hmmmm... I'm trying to understand what you mean. I know that I could do something likeTimer = New MyTimer(1000, True, Function() Return TextBox1.Text <> String.Empty)but I don't know where that return value would go or how to handle it.
BASnappl
[Events can't be functions](http://msdn.microsoft.com/en-us/library/xzfs26c9.aspx) as far as I know. Maybe Dean means the event should have a ByRef Boolean argument for the return value? `Public Event Func(ByRef bOut As Boolean)` Then define your lambda Sub with a Boolean argument
MarkJ