28 Aug 2009 @ 12:20 AM 

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 :D

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
Posted By: Jeroen De Dauw
Last Edit: 29 Aug 2009 @ 01:46 PM

EmailPermalink
Tags


 

Responses to this post » (None)

 
Post a Comment

XHTML: You can use these tags: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>


 Last 50 Posts
 Back
Change Theme...
  • Users » 4744
  • Posts/Pages » 197
  • Comments » 156
Change Theme...
  • VoidVoid « Default
  • LifeLife
  • EarthEarth
  • WindWind
  • WaterWater
  • FireFire
  • LightLight

About me



    No Child Pages.