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.
[cc lang=”csharp” width=”607″] 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);
}
}
[/cc]
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.
[cc lang=”vbnet” width=”607″] 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[/cc]