Simplify ASP.Net StreamWriter for appendtext to file Is there anyway to simplify this code so there aren't so many redundant w = File.AppendText(filePath) and w.flush() and w.close() in each if statement?

Anything else I have tried ends up locking the file, and crashing the site because to text file is in use by another process.

Public filePath As String = "c:\inetpub\logs\login.txt"
Public w As StreamWriter

Protected Sub LoginUser_LoginError(ByVal sender As Object, ByVal e As System.EventArgs) Handles LoginUser.LoginError

    Dim UserName As TextBox = DirectCast(LoginUser.FindControl("UserName"), TextBox)

    Dim CurrentUser As MembershipUser = Membership.GetUser(LoginUser.UserName)

    Dim logLoginError As New Thread(
        Sub()
            If (CurrentUser IsNot Nothing) Then

                If (CurrentUser.IsLockedOut = True) Then
                    w = File.AppendText(filePath)
                    w.WriteLine(CurrentUser.ToString &amp; "'s (" &amp; IPAddress &amp; ") account is locked out - " &amp; DateAndTime.Now)
                    w.Flush()
                    w.Close()
                ElseIf (CurrentUser.IsApproved = False) Then
                    w = File.AppendText(filePath)
                    w.WriteLine(CurrentUser.ToString &amp; " (" &amp; IPAddress &amp; ") is attempting to login, but the account is disabled - " &amp; DateAndTime.Now)
                    w.Flush()
                    w.Close()
                Else
                    w = File.AppendText(filePath)
                    w.WriteLine(CurrentUser.ToString &amp; "'s (" &amp; IPAddress &amp; ") login credentials failed - " &amp; DateAndTime.Now)
                    w.Flush()
                    w.Close()
                End If

            Else
                w = File.AppendText(filePath)
                w.WriteLine("Unknown user " &amp; UserName.Text &amp; " (" &amp; IPAddress &amp; ") attempting to login - " &amp; DateAndTime.Now)
                w.Flush()
                w.Close()
            End If
        End Sub
    )
    logLoginError.Start()

End Sub
Careful with naming.

UserName is an identifier for a TextBox, but it's named like a String (which contains a user's name).

I'd declare and assign UserName like this:

Dim UserName As String = DirectCast(LoginUser.FindControl("UserName"), TextBox).Text


And now the rest of the code can refer to a string that represents a user's name without any further conversions.



logLoginError is an identifier for a Thread, but it's named like a Task (which runs on a thread).

loggerThread would be a more accurate name for it.



This code:

w.WriteLine(CurrentUser.ToString &amp; "'s (" &amp; IPAddress &amp; ") account is locked out - " &amp; DateAndTime.Now)


Is using way more String instances than you'd think:


CurrentUser.ToString()
"'s ("
IPAddress
") account is locked out - "
DateAndTime.Now[.ToString]


And also:


The string resulting from CurrentUser.ToString() &amp; "'s ("
The string resulting from CurrentUser.ToString() &amp; "'s (" &amp; IPAdress
The string resulting from CurrentUser.ToString() &amp; "'s (" &amp; IPAdress &amp; ") account is locked out - "
The string resulting from CurrentUser.ToString() &amp; "'s (" &amp; IPAdress &amp; ") account is locked out - " &amp; DateAndTime.Now


Are you sure DateAndTime isn't supposed to be DateTime? Also there's a typo in IPAdress, should be IPAddress.

To fix the number of String objects being created, you can use String.Format instead of inline concatenations - remember that String is an immutable class, "appending to it" doesn't really append anything, it just creates a new instance.

This would be cleaner and more efficient (multiline to avoid horizontal scrolling):

w.WriteLine(String.Format("{0}'s ({1}) account is locked out - {2}", 
                          CurrentUser.ToString, IPAddress, DateAndTime.Now))