-1

I want to add column from one table to another. Here, colName is string variable, which is extracted from hard-coded parameter provided by developer. so chances of colName being incorrect is very less. I want to avoid exception raised due to wrong colName.

Which is the best way to achieve same? I thought of two options below.

if(_table.Columns.Contains(colName))
{
     AddColumnToTable(_table.Columns[colName]);
}

OR

try
{
     AddColumnToTable(_table.Columns[colName]);   
}
catch { }
  • 6
    Better to test and then to act, than to act blindly and then to catch. And faster too normally (if the possible failure is quite common) – xanatos Jun 26 '15 at 12:29
  • Does `AddColumnToTable()` throw an exception? If not, then the try catch won't be helpfull... – Mivaweb Jun 26 '15 at 12:29
  • @Mivaweb I'll say that it is more a `_table.Columns[]` that throws. – xanatos Jun 26 '15 at 12:30
  • 1
    I agree with @xanatos. The notable exception is race conditions, example: `if (File.Exists(f)) File.Delete(f)`. – Kuba Wyrostek Jun 26 '15 at 12:31
  • @KubaWyrostek And even there, sometimes I would probably prefer to duplicate the effort and do both. – xanatos Jun 26 '15 at 12:33
  • I am more concern about exception by _table.Columns[]. Also the scope of error is very less as the string passed will be added by me only. So my though was, If the chances of exception are very less, why to check it every time. Anyways I m nt gonna do anything if exception occurs, it is gonna get supressed – Vishal Sonavane Jun 26 '15 at 12:37
  • 1
    Eric Lippert has a [great article on exceptions](http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx). This would fall under 'boneheaded', – Jonesopolis Jun 26 '15 at 12:50
  • There is no *best* way. But [empty catch](http://stackoverflow.com/q/1234343/1997232) is definitely rare case to be a [good idea](http://stackoverflow.com/q/4692056/1997232). Otherwise, this question was [asked](http://stackoverflow.com/q/6186340/1997232) and [asked](http://stackoverflow.com/q/17335217/1997232) many times. Use search maybe? – Sinatr Jun 26 '15 at 13:14

4 Answers4

4

First of all, the whole world died a little inside when this happened:

catch { }

But I digress...


The real question isn't whether you should use if vs. try, the real question is:

If the column can't be added, then what?

If this is an expected scenario and basically "not a big deal" and the logic can continue just fine without adding the column, then if is definitely the way to go. As mentioned in a comment on the question, "test and then act".

If, however, this is not an expected scenario and it "is a big deal" and the logic can't meaningfully continue, throw an exception. Don't catch it and ignore it. Let the consuming code (at the application level) catch it and handle it, likely notifying a user or logging the error or perhaps even attempting to correct it in some way.

(Or if this code is at the application level, catch it here and handle it. The point being that handling is different from catching. The latter is a simple catch block, the former is the custom logic required to respond to the error in a meaningful way.)


You can even add further information to the exception. For example:

try
{
    // perform some operation
}
catch (SpecificException ex)
{
    throw new CustomException("Failed to perform Operation X in the context of Y.", ex);
}

This can be very valuable when diagnosing a production system where you can't attach a debugger. Specific exception types, helpful error messages, and of course the technical details of the original exception are all necessary tools.

David
  • 208,112
  • 36
  • 198
  • 279
  • Another day, another subjective question on exceptions and another answer that I utterly disagree with. Do not just let the code throw an exception that some other aspect of the code then has to second guess the cause and act upon. In this case, if it is a huge deal that the column exist, then throw your own "mandatory column x doesn't exist and couldn't be added to the table" exception, that details as much possible why this is a big deal, inside the else, rather than letting a vague "element doesn't exist" exception bubble away from its context. – David Arno Jun 26 '15 at 12:39
  • @DavidArno: I think we're again failing to understand each other's points, and I suspect we agree with each other more than you think. Adding meaningful context to an exception is *definitely* a good idea, and I don't encourage otherwise in my answer or anywhere. The point is that an exception should bubble up to consuming code if this code isn't the right level of abstraction to handle the error. Whether that exception is built-in or custom makes little difference. – David Jun 26 '15 at 12:42
  • I only want to stop code from breaking and continue execution even if exception occure – Vishal Sonavane Jun 26 '15 at 12:44
  • @VishalSonavane: In that case it sounds like this is an expected scenario and you want to use an `if` statement to test the condition. Think of it this way... If the inability to add the column isn't an *exceptional* event, then an exception shouldn't be involved. Never use exceptions to drive logic, only use them to halt execution of a particular operation. – David Jun 26 '15 at 12:46
  • I died a little when I saw `throw new ...` then noticed this is tagged C#, not C++ – Aykhan Hagverdili Apr 09 '19 at 15:12
0

Based solely on the information you provide there is only one conclusion :

if(_table.Columns.Contains(colName))
{
    AddColumnToTable(_table.Columns[colName]);
}

because the alternative is an exception handling with an empty catch block. This basically means that it really IS your intent not to do anything with the condition that the colName is not found. However if your code is stripped down to an arbitrary example and you just cleared the catch block to make the post more condense, the conclusion might be different.

Doing this :

catch { }

Should anyhow be avoided.

Based on this statement :

Here, colName is string variable, which is extracted from hard-coded parameter provided by developer.

You could opt to include an assertion in your code instead of an exception because passing the wrong column name can be considered an application bug. Because you don't really want to foresee an exception I think an assert is quite a good alternative. That would yield following code :

if(_table.Columns.Contains(colName))
{
    AddColumnToTable(_table.Columns[colName]);
}
else Debug.Assert(false,"Column name is wrong, please correct calling code.");

or more condense :

Debug.Assert(_table.Columns.Contains(colName),
             "Column name is wrong, please correct calling code.");
AddColumnToTable(_table.Columns[colName]);
Philip Stuyck
  • 7,344
  • 3
  • 28
  • 39
  • NO I dont want to do anything in catch block. I want to continue execution even if exception occurs. Basically the chances of occurring exception is very less. will that affect? – Vishal Sonavane Jun 26 '15 at 12:47
  • Well using an assert only affects a debug build, your production build will continue. If you don't even want an assert in a debug build then the if statement is the best for you. – Philip Stuyck Jun 26 '15 at 12:57
0

Never use try/catch if you can use if. It's the matter of code readability and maintainability.

If you value performance, however, you might consider possibility of failure in this scenario. Your library might do something like this:

void AddColumn(string columnName, Column column)
{
    if(this.Columns.Contains(columnName))
         throw new DuplicateColumnException();
    else
         this.Columns.Add(columnName, column);
}

Which would mean that your if would add second Contains invocation and depending on its complexity throwing an exception might be better than invoking second check.

The best way to resolve this would be finding the method that sets the column:

void SetColumn(string columnName, Column column)
{
    if(this.Columns.Contains(columnName))
         this.Columns[columnName] = column;
    this.Columns.Add(columnName, column);
}

or simply:

void SetColumn(string columnName, Column column)
{
    this.Columns[columnName] = column;
}
Ryszard Fiński
  • 452
  • 3
  • 7
0

In the past I have written games in which performance is incredibly important. I found that in cases where the "catch" statement was being hit, there was a serious performance impact. Things were really slow. i guess this is because it has to capture all details about what went wrong and try and output that in the catch statement, all very inefficient stuff. I would try and avoid using a try statement wherever you can. It is always best to do an if statement to capture this sort of thing, especially if performance is important.

James Kirk
  • 24
  • 2