HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

Should this be written with exception handling instead of nested if-thens?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thishandlingwrittenexceptionwithinsteadnestedshouldthens

Problem

I have a function that iterates through a list of file updates, and for each file, if updates are needed, download the file from the server, make the updates, and upload the updated version. Each update gets marked with a string identifying the result of the update operation.

There are several steps in the update process where I would not want updating to continue for that file. Right now I have a lot of nested if-then statements, so that the iteration won't proceed any further for that file and will just mark the issue and go on to the next file. Code as follows:

```
public void updateAllFiles()
{
foreach (string fileName in updateList.fileNameList())
{
updatesForFile = updateList.updatesForFile(fileName);
hyperlinkEntry = hyperlinkList.getHyperlinkEntry(fileName);
if (hyperlinkEntry == null)
{
updatesForFile.markResultAllRows("File not found on server");
}
else
{
hyperlink = hyperlinkEntry.hyperlink;
if (updatesForFile.updateNeeded())
{
if (!po.checkout(hyperlink))
{
updatesForFile.markResultAllRows("checked out to other user");
}
else
{
downloadPath = downloadFolder + fileName;
downloadResult = dl.download(hyperlink, downloadPath);
if (downloadResult == false)
{
updatesForFile.markResultAllRows("download failed");
}
else
{
file = new FileObject(xlApp, downloadPath, updatesForFile);
if (file == null)
{
updatesForFile.markResultAllRows("unable to open");

Solution

Maybe simple loop logic can serve your purposes better. If what you want is to ignore the rest of the loop body if a condition is met, the continue statement will follow immediately with the next iteration.

foreach (string fileName in updateList.fileNameList())
{
    updatesForFile = updateList.updatesForFile(fileName);
    hyperlinkEntry = hyperlinkList.getHyperlinkEntry(fileName);
    if (hyperlinkEntry == null)
    {
        updatesForFile.markResultAllRows("File not found on server");
        continue;
    }
    hyperlink = hyperlinkEntry.hyperlink;
    if (updatesForFile.updateNeeded())
    {
        if (!po.checkout(hyperlink))
        {
            updatesForFile.markResultAllRows("checked out to other user");
            continue;
        }
        downloadPath = downloadFolder + fileName;
        downloadResult = dl.download(hyperlink, downloadPath);
        if (downloadResult == false)
        {
            updatesForFile.markResultAllRows("download failed");
            continue;
        }
        file = new FileObject(xlApp, downloadPath, updatesForFile);
        if (file == null)
        {
            updatesForFile.markResultAllRows("unable to open");
            continue;
        }
        foreach (FeatureUpdate featureUpdate in updatesForFile.getFeatureUpdateList())
        {
            resultValue = featureUpdate.getColumnValue("result");
            if (!resultValue.Contains("updated"))
            {
                file.update(featureUpdate, updateMethods.lowFeatureLength);
                updatesForFile.markResult(featureUpdate.getIndex(), "updated");
            }
        }
        file.close();
        updateList.save();
    }
}


If what you need is to exit altogether when a condition is met, then I'd favor return, maintaining the same logic as the continue example. This would also give a lot more flexibility to refactor your code into functions and even probably use of kind of error codes, if it gets to that point.

Exceptions are very useful, but they are very expensive, so unless you don't really care about the performance of this code, I'd favor a solution based in flow control any day of the week.

Code Snippets

foreach (string fileName in updateList.fileNameList())
{
    updatesForFile = updateList.updatesForFile(fileName);
    hyperlinkEntry = hyperlinkList.getHyperlinkEntry(fileName);
    if (hyperlinkEntry == null)
    {
        updatesForFile.markResultAllRows("File not found on server");
        continue;
    }
    hyperlink = hyperlinkEntry.hyperlink;
    if (updatesForFile.updateNeeded())
    {
        if (!po.checkout(hyperlink))
        {
            updatesForFile.markResultAllRows("checked out to other user");
            continue;
        }
        downloadPath = downloadFolder + fileName;
        downloadResult = dl.download(hyperlink, downloadPath);
        if (downloadResult == false)
        {
            updatesForFile.markResultAllRows("download failed");
            continue;
        }
        file = new FileObject(xlApp, downloadPath, updatesForFile);
        if (file == null)
        {
            updatesForFile.markResultAllRows("unable to open");
            continue;
        }
        foreach (FeatureUpdate featureUpdate in updatesForFile.getFeatureUpdateList())
        {
            resultValue = featureUpdate.getColumnValue("result");
            if (!resultValue.Contains("updated"))
            {
                file.update(featureUpdate, updateMethods.lowFeatureLength);
                updatesForFile.markResult(featureUpdate.getIndex(), "updated");
            }
        }
        file.close();
        updateList.save();
    }
}

Context

StackExchange Code Review Q#43960, answer score: 4

Revisions (0)

No revisions yet.