Raising a error when trying to save a empty listbox

Question

When the user clicks save, and there is nothing in the listbox, i want to raise an error. I figure I use a try catch block like so:

           try
        {
            //when you go to save, and the list box is empty, you should get an error message
            if (dataListBox.Items.Equals(null))
                throw new Exception();

            //i wanted to save on the form_close event, so i put the save data into a method and just call the method from this event 
            this.save();
        }
        catch (Exception err)
        {
            //spits out the errors if there are any
            MessageBox.Show(err.Message, "List Box is empty", MessageBoxButtons.OK, MessageBoxIcon.Error);
        }

But this isnt working for me, it still saves an no message box comes up.

Answer 1

Try to add this:

if(dataListBox.Items.Count==0)
            throw new Exception();

Btw, try to define your own Exception class here, for example EmptyListException, then you'll be sure that you catch exactly what you want to catch. Now this code will show the same MessageBox for the exception raised in the "Save" method as well.

Answer 2

dataListBox.Items.Count == 0

IMO, you don't really need an exception in this case, especially if you use an untyped exception (other exceptions than yours could be caught by the catch block)

Answer 3

you need to check:

if(dataListBox.Items.Count ==0)
   throw new Exception();

Answer 4

You should use .Count() instead.

if (dataListBox.Items.Count < 1)
    throw new Exception();

Answer 5

Don't do so at all. Compare:

try
{
//when you go to save, and the list box is empty, you should get an error message
    if (dataListBox.Items.Count != 0)
        throw new Exception("Please add at least one item to the list.");

    this.save();
}
catch (Exception err)
{
    //spits out the errors if there are any
    MessageBox.Show(err.Message, "List Box is empty", MessageBoxButtons.OK, MessageBoxIcon.Error);
}

With:

    if (dataListBox.Items.Count != 0)
    {
        MessageBox.Show("Please add at least one item to the list.", "List Box is empty", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
    else
    {
        this.save();
    }

1. It's more efficient to not use exceptions for control flow. Yeah, efficiency isn't everything, but there's no point in premature pessimisation either.
2. Validation failures are not exceptions, because we expect them. If the code had got to the point where it was trying to save and that was logically impossible due to the lack of data then that might be an exception (but one that should have been caught in validation in higher up code - specifically this code). If saving fails because of a filesystem error, db connection error etc., then that's an exception. User validation though is something that is not an exception and should only be reported as such if we need to talk to a higher level of code (again, that code should have caught it if at all possible through control-flow mechanisms).

3. It makes it easier to spot the logical bug. Because we've separated exceptional and non-exceptional cases, we can see that we aren't looking for the real possible exception. As is, if you had a user fill in the list properly, but the save failed because of a real exception, you are going to say "List box is empty" in your message box's caption, which will confuse the user. This becomes plainer now, and it's easier to fix that bug:

   if (dataListBox.Items.Count != 0)
   {
       MessageBox.Show("Please add at least one item to the list.", "List Box is empty", MessageBoxButtons.OK, MessageBoxIcon.Error);
   }
   else
   {
       try
       {
           this.save();
       }
       catch(Exception ex)
       {
            MessageBox.Show("Saving failed.\n Technical details:\n" + ex.Message, "Saving Failed", MessageBoxButtons.OK, MessageBoxIcon.Error);
       }
   }