.Net C# to VB + refactoring

From the archive: written in 2009 and kept for the record. Much of it is out of date.

While translating some of the C# code of MyDownloader to VB.Net for the .Net download library, I’ve already come across quite some awkward code. IMHO, the underneath example could go into the code horrors section of The Code Project.

        private void RestartDownload()
        {
            int currentTry = 0;
            Stream stream;
            RemoteFileInfo newInfo;

            try
            {
                do
                {
                    lastError = null;

                    SetState(DownloaderState.Preparing);

                    currentTry++;
                    try
                    {
                        newInfo = defaultDownloadProvider.GetFileInfo(this.ResourceLocation, out stream);

                        break;
                    }
                    catch (Exception ex)
                    {
                        lastError = ex;
                        if (currentTry < Settings.Default.MaxRetries)
                        {
                            SetState(DownloaderState.WaitingForReconnect);
                            Thread.Sleep(TimeSpan.FromSeconds(Settings.Default.RetryDelay));
                        }
                        else
                        {
                            return;
                        }
                    }
                }
                while (true);
            }
            finally
            {
                SetState(DownloaderState.Prepared);
            }

            try
            {
                // check if the file changed on the server
                if (!newInfo.AcceptRanges ||
                    newInfo.LastModified > RemoteFileInfo.LastModified ||
                    newInfo.FileSize != RemoteFileInfo.FileSize)
                {
                    this.remoteFileInfo = newInfo;
                    StartSegments(this.RequestedSegments, stream);
                }
                else
                {
                    if (stream != null)
                    {
                        stream.Dispose();
                    }

                    RunSegments();
                }
            }
            catch (ThreadAbortException)
            {
                throw;
            }
            catch (Exception ex)
            {
                lastError = ex;
                SetState(DownloaderState.EndedWithError);
            }
        }

Why? It uses breaks and returns in the middle of the code, like if C# is some lame generic language without selection structures. The use of the try-catch bloks also seems odd. Also have a look at the condition for the do while loop – lol. The only thing missing are a few GoTo statements, which would have convinced me to print this out and torture fellow geeks with 😀

I started off translating this literally, but got so annoyed by the bad coding practice I decided to attempt to rewrite it. I say ‘attempt’, cause code written like that is not easy to understand, esp for people who are used to ‘decent’ structured code. I splitted the code into 2 subs instead of one, removed all breaks, returns, the insane do while true and put only the required code in try catches.

        Private Sub RestartDownload()
            Dim tryNr As Int32 = 0
            Dim stream As Stream
            Dim newInfo As RemoteFileInfo
            Dim reachedMaxTries, hasError As Boolean

            Do
                m_lastError = Nothing

                Me.SetState(DownloadState.Preparing)

                Try
                    newInfo = Me.ProtocolProvider.GetFileInfo(Me.FileLocation, stream)
                Catch ex As Exception
                    m_lastError = ex
                End Try

                hasError = m_lastError IsNot Nothing

                If hasError Then
                    tryNr += 1
                    reachedMaxTries = tryNr >= Me.Settings.MaxRetries
                End If

                If hasError And Not reachedMaxTries Then
                    Me.SetState(DownloadState.WaitingForReconnect)
                    Thread.Sleep(Me.Settings.RetryDelay)
                End If
            Loop Until Not hasError Or reachedMaxTries

            If hasError Then
                Me.SetState(DownloadState.EndedWithError)
            Else
                Me.SetState(DownloadState.Prepared)
                Me.RestartSegments(newInfo, stream)
            End If
        End Sub

        Private Sub RestartSegments(ByVal newInfo As RemoteFileInfo, ByVal stream As Stream)
            If Not newInfo.AcceptRanges Or newInfo.ModifyDateTime > Me.FileInfo.ModifyDateTime Or newInfo.FileSize <> Me.FileInfo.FileSize Then
                m_fileInfo = newInfo
                Me.AttemptToStartSegements(stream)
            Else
                If stream IsNot Nothing Then stream.Dispose()
                Me.RunSegments()
            End If
        End Sub

Jeroen De Dauw is founder and CEO of Professional Wiki and builds NeoWiki. More about me. Reply by email or on Mastodon, Bluesky or X.