views:

39

answers:

4

hey guy, have onather problem.

I am trying to raise an exception if a user clicks the delete button but doesnt select a item in the listbox.

This is what i have:

        try
        {
            if (dataListBox.SelectedIndex == null)
                throw new Exception;

            //deletes selected items
            dataListBox.Items.RemoveAt(dataListBox.SelectedIndex);
            isDirty = true;
        }
        catch (Exception err)
        {
            //spits out the errors if there are any
            MessageBox.Show(err.Message, "Enter something into the txtbox", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }
+2  A: 

You don't really need to throw an exception here, you could do the following:

if (dataListBox.SelectedIndex == -1)
{
    MessageBox.Show(err.Message, "Enter something into the txtbox", MessageBoxButtons.OK, MessageBoxIcon.Error)
}
else
{
    //deletes selected items
    dataListBox.Items.RemoveAt(dataListBox.SelectedIndex);
    isDirty = true;
}
Patrick McDonald
+1  A: 

Use

SelectedIndex == -1

to know that nothing is selected.

Timores
thanks dude, did the trick
Codie Vincent
You're welcome, but I agree with the other answers. An exception is not the best solution here, and, if it were, a specific exception type is recommended.
Timores
+2  A: 

You should not use exceptions to check values.

Do something like:

if (dataListBox.SelectedIndex >= 0)
{
  //standard flow code
}
else
{
  MessageBox.Show("Enter something into the txtbox",_
MessageBoxButtons.OK, MessageBoxIcon.Error); 
} 

instead.

zgorawski
+1  A: 

The test is wrong, check for -1. And it is missing the parentheses, throw new Exception().

This is however very inappropriate code in many different ways. That starts by throwing Exception instead of a specific exception derived class object. Your catch clause will catch every exception, including ones thrown by other mishaps like a bug in your code. You tell the user to enter something when she actually did.

Furthermore, exceptions should only be used for exceptional circumstances. There is nothing exceptional about the user forgetting to do something. Just display the message in the if() statement, do the rest in the else clause.

Furthermore, you should not even allow the user to fall into this trap. Only set the Delete button's Enabled property to true when you see that she's selected something.

Hans Passant