[RESOLVED] Long methods

[chris128]

I understand that it is good practice to break up your application's code into separate blocks - so for example you would not just have your entire code in a button click event, you would break up the parts of the procedure into methods that call each other. Normally I follow this practice but in my current little project (creating an SMTP server) I am finding that difficult. I'll explain...

I have a method that is called each time a remote server connects to my SMTP server and tries to pass a message to it, this method needs to read the data that the remote server has sent and then respond to the remote server appropriately and store certain parts of the data the remote server sent (ie the sender address of the email message, the recipient address, the body of the email etc). Now you would think that this is quite easy to separate into different methods as you would just have one function that reads the data, then a quick IF statement to see what the command was that the remote server sent, and then a separate procedure for each of the possible commands that takes an appropriate action.

However, because the actions of each of these parts all need to access so many things that are declared in the original method, it seems pointless just passing these items around through parameters in functions/subs just for the sake of the code looking a bit neater. Its kind of hard to explain but it seems to me that without either declaring all these things at class level (which looks pretty naff to me) or passing loads of object references around between each method, then there isnt really any obvious way of separating this code.

Maybe its just me - maybe this method is no where near what you would consider "too long" and maybe the amount of objects I would need to pass to each method if I separated it all would not be considered anything out of the ordinary but just have a look at the code below and see what you think... As you can see, I have defined separate procedures for some things, such as the WriteToClientStream and SMTPSendMessage but I'm struggling to find any more things that are self contained enough to split out

''' <summary>
''' Handles the session and command processing for each connected client
''' </summary>
''' <param name="AcceptedTcpClient">The TcpClient object that represents the connected client</param>
Private Sub ManageSession(ByVal AcceptedTcpClient As Object)
 
        Dim RemoteClient As TcpClient = DirectCast(AcceptedTcpClient, TcpClient)
        Dim RemoteIP As String = RemoteClient.Client.RemoteEndPoint.ToString
        Dim ClientStream As NetworkStream = RemoteClient.GetStream
 
        'Respond to the connection request and raise the ClientConnected event
        WriteToClientStream(ClientStream, "220 Prototype SMTP Server , ready at " & Now.ToString)
        RaiseEvent ClientConnected(New SMTPSessionEventArgs(RemoteIP, String.Empty, String.Empty))
 
 
        'Flags that are used to track the current progress of the session
        Dim QuitCommandReceived As Boolean = False
        Dim InDataCommand As Boolean = False
        Dim MessageSent As Boolean = False
 
        'Email attributes populated by received SMTP commands
        Dim SenderAddress As String = String.Empty
        Dim RecipientAddresses As New List(Of String)
        Dim Subject As String = String.Empty
        Dim MessageBody As String = String.Empty
 
        'Will contain the string of data received from the remote client
        Dim ReceivedStringBuilder As New StringBuilder
 
        Do While QuitCommandReceived = False
            Do
                Dim size As Integer = 0
                Dim DataAsBytes(1024) As Byte
                Do While ClientStream.DataAvailable
                    size = ClientStream.Read(DataAsBytes, 0, DataAsBytes.Length)
                    Dim ShortDataAsString As String = System.Text.Encoding.ASCII.GetString(DataAsBytes, 0, size)
                    ReceivedStringBuilder.Append(ShortDataAsString)
                Loop
 
                If InDataCommand Then
                    If ReceivedStringBuilder.ToString.EndsWith(vbCrLf & "." & vbCrLf) Then
                        WriteToClientStream(ClientStream, "250 Message queued for delivery.")
                        MessageBody = ReceivedStringBuilder.ToString.Remove(ReceivedStringBuilder.Length - 3, 3)
                        SMTPSendMessage(SenderAddress, RecipientAddresses, Subject, MessageBody, RemoteIP)
                        InDataCommand = False
                        SenderAddress = String.Empty
                        RecipientAddresses.Clear()
                        Subject = String.Empty
                        MessageBody = String.Empty
                        MessageSent = True
                        Exit Do
                    End If
                ElseIf InDataCommand = False Then
                    If ReceivedStringBuilder.ToString.EndsWith(vbCrLf) Then
                        Exit Do
                    End If
                End If
            Loop 'Start listening for new data on the stream again
 
            If MessageSent Then
                MessageSent = False
            Else
                Dim CommandEntered As String = String.Empty
                CommandEntered = ReceivedStringBuilder.ToString
                RaiseEvent CommandReceived(New SMTPSessionEventArgs(RemoteIP, CommandEntered, String.Empty))
 
                '-------- HELO Command ---------
                If String.Compare(CommandEntered, "HELO" & vbCrLf, True) = 0 Then
                    WriteToClientStream(ClientStream, "250 HELO From " & RemoteClient.Client.LocalEndPoint.ToString)
 
                    '--- EHLO Command ---
                ElseIf CommandEntered.StartsWith("EHLO ", StringComparison.CurrentCultureIgnoreCase) Or String.Compare(CommandEntered, "EHLO" & vbCrLf, True) = 0 Then
                    WriteToClientStream(ClientStream, "250 OK")
 
                    '--- MAIL FROM Command ---
                ElseIf CommandEntered.StartsWith("MAIL FROM:", StringComparison.CurrentCultureIgnoreCase) Then
                    If Not SenderAddress = String.Empty Then
                        WriteToClientStream(ClientStream, "503 5.5.2 Sender already specified")
                    Else
                        Dim RequestedSenderAddress As String = CommandEntered.Substring(CommandEntered.IndexOf(":") + 1).Trim
                        If RequestedSenderAddress.Contains("@") = False Then
                            WriteToClientStream(ClientStream, "501 5.5.4 Invalid Address")
                        Else
                            SenderAddress = RequestedSenderAddress
                            WriteToClientStream(ClientStream, "250 2.1.0 " & SenderAddress & "....Sender OK")
                        End If
                    End If
 
                    '--- RCPT TO Command ---
                ElseIf CommandEntered.StartsWith("RCPT TO:", StringComparison.CurrentCultureIgnoreCase) Then
                    If SenderAddress = String.Empty Then
                        WriteToClientStream(ClientStream, "503 5.5.2 Need Mail From: first")
                    Else
                        Dim RequestedRecipientAddress As String = CommandEntered.Substring(CommandEntered.IndexOf(":") + 1).Trim
                        If RequestedRecipientAddress.Contains("@") = False Then
                            WriteToClientStream(ClientStream, "501 5.5.4 Invalid Address")
                        Else
                            RecipientAddresses.Add(RequestedRecipientAddress)
                            WriteToClientStream(ClientStream, "250 2.1.5 " & RequestedRecipientAddress)
                        End If
                    End If
 
                    '--- DATA Command ---
                ElseIf String.Compare(CommandEntered, "DATA" & vbCrLf, True) = 0 Then
                    If SenderAddress = String.Empty Then
                        WriteToClientStream(ClientStream, "503 5.5.2 Need mail command.")
                    ElseIf RecipientAddresses.Count < 1 Then
                        WriteToClientStream(ClientStream, "503 5.5.2 Need Rcpt command.")
                    Else
                        WriteToClientStream(ClientStream, "354 Start mail input; end with <CRLF>.<CRLF>")
                        InDataCommand = True
                    End If
 
                    '--- AUTH LOGIN Command ---
                    ElseIf String.Compare(CommandEntered, "AUTH LOGIN" & vbCrLf, True) = 0 Then
                    'Havent got this far yet...
 
                    '--- RSET Command ---
                ElseIf String.Compare(CommandEntered, "RSET" & vbCrLf, True) = 0 Then
                    SenderAddress = String.Empty
                    RecipientAddresses.Clear()
                    Subject = String.Empty
                    MessageBody = String.Empty
                    InDataCommand = False
                    WriteToClientStream(ClientStream, "250 2.0.0 Resetting")
                    '--- QUIT Command ---
                ElseIf String.Compare(CommandEntered, "QUIT" & vbCrLf, True) = 0 Then
                    QuitCommandReceived = True
                    RemoteClient.Close()
                    '--- Unknown Command ---
                Else
                    WriteToClientStream(ClientStream, "500 5.3.3 Unknown Command: " & CommandEntered)
                End If
            End If
 
            'Clear out the command string ready for next command
            ReceivedStringBuilder.Remove(0, ReceivedStringBuilder.Length)
 
        Loop 'Start processing commands that are sent from the client again (unless QUIT has been sent)
End Sub