Best way to throw exceptions on query attempt c#

Question

Hi I'm developing a winform application using C# and the entity framework (linq to entities). Suppose the following escenario:

In a method of some class, I set and object of values with form values

 private void agrega_cliente_Click(object sender, EventArgs e)
 {
        cliente = new _Cliente();

        try
        {
            cliente.nombres = nom_cliente.Text;
            cliente.apellidoP = apellidoP_cliente.Text;
            cliente.apellidoM = apellidoM_cliente.Text;
            cliente.fechaNacimiento = fechaNacimientoPicker.Value.Date;

            if (operaciones.AgregaCliente(cliente, referencias))
            {
                MessageBox.Show("Cliente Agregado");
                this.Close();
            }
        }
        catch(Exception ex)
        {
            MessageBox.Show(ex.ToString());
        }
 }

Note that the assigment and calling to method "AgregaCliente" is between a try and catch, so if an exception is triggered a MessageBox will show it.

Then in other class, I have AgregaCliente method which insert values in a database.

 public bool AgregaCliente(_Cliente cliente, ArrayList refes)
 {
        try
        {
            Cliente cli = new Cliente()
            {
                Nombres = cliente.nombres,
                ApellidoP = cliente.apellidoP,
                ApellidoM = cliente.apellidoM,
                FechaNac = cliente.fechaNacimiento
            };
            if (NombreExiste(cli))
                context.clientes.AddObject(cli);
            else
                throw new System.ArgumentException("El usuario ya existe");
            if (refes.Count != 0)
            {
                foreach (_Referencia elem in refes)
                    context.referencias_personales.AddObject(AgregaReferencia(elem));
            }
            context.SaveChanges();
        }
        catch (Exception ex)
        {
            return false;
        }
        return true;
 }

Inside this method there is a call to "NombreExiste()" which checks that the user isn't already inserted, if the user exists an exception is thrown.

So the problem here is that if an exception is thrown in "AgregaCliente" method, I want this exception to be catched by "agrega_cliente_Click()" method, so the user knowns what originated the problem. I hope you understand what I'm trying to do.

Thanks

Answer 1

The problem is that your AgregaCliente() method is catching all exceptions and simply swallowing them. Instead of catching all exceptions via:

    catch (Exception ex)
    {
        return false;
    }

You should only catch specific Exceptions you can handle and let the others pass up the call chain. However, you should know that throwing exceptions is very "expensive" for a program. C# does a lot of work behind the scenes when an exception is thrown. A better solution may be to use a return code to indicate to callers of the AgregaCliente() method the status. For example:

public enum AgregaClienteStatus
{
  Success = 0;
  ClientAlreadyExists = 1;
  Other = ??;  // Any other status numbers you want
}

 public AgregaClienteStatus AgregaCliente(_Cliente cliente, ArrayList refes)
 {

            Cliente cli = new Cliente()
            {
                Nombres = cliente.nombres,
                ApellidoP = cliente.apellidoP,
                ApellidoM = cliente.apellidoM,
                FechaNac = cliente.fechaNacimiento
            };
            if (NombreExiste(cli))
                context.clientes.AddObject(cli);
            else
                return AgregaClienteStatus.ClientAlreadyExists
            if (refes.Count != 0)
            {
                foreach (_Referencia elem in refes)
                    context.referencias_personales.AddObject(AgregaReferencia(elem));
            }
            context.SaveChanges();


        return AgregaClientStatus.Success;
 }

Of course, this functionality could also be achieved using constant integers if you don't like enums.

You can then use that return status to indicate information to the user without the expense of an exception:

  var result = AgregaClient(cliente, refes);
  switch (result)
  {
    case AgregaClientStatus.Success:
         // Perform success logic
         break;
    case AgregaClientStatus.ClientAlreadyExists:
         MessageBox.Show("Client already exists");
         break;
    // OTHER SPECIAL CASES
    default:
         break;
   }

}

Answer 2

Simply get rid of your try/catch inside the AgregaCliente() method and the exception will automatically bubble-up.

public bool AgregaCliente(_Cliente cliente, ArrayList refes) 
{ 
    Cliente cli = new Cliente() 
    { 
        Nombres = cliente.nombres, 
        ApellidoP = cliente.apellidoP, 
        ApellidoM = cliente.apellidoM, 
        FechaNac = cliente.fechaNacimiento 
    }; 
    if (NombreExiste(cli)) 
        context.clientes.AddObject(cli); 
    else 
        throw new System.ArgumentException("El usuario ya existe"); 
    if (refes.Count != 0) 
    { 
        foreach (_Referencia elem in refes) 
            context.referencias_personales.AddObject(AgregaReferencia(elem)); 
    } 
    context.SaveChanges(); 

    return true; 
}