0

I'm developing a C# .NET Framework 4.0 library.

I have this code:

public static byte GetBatchStatus(string connString)
{
    if (string.IsNullOrEmpty(connString))
        throw new ArgumentNullException("connString");

    byte status;

    using (System.Data.SqlClient.SqlConnection conn = new System.Data.SqlClient.SqlConnection(connString))
    {
        conn.Open();

        SqlCommand cmd = new SqlCommand();

        cmd.CommandText = GetBatchStatusValueSQL;
        cmd.CommandType = CommandType.Text;
        cmd.Connection = conn;

        object o = cmd.ExecuteScalar();
        // Throws an ArgumentNullException if o is null.
        if (o == null)
            throw new ArgumentNullException("o");

        status = Convert.ToByte(o);
    }

    return status;
}

cmd.ExecuteScalar(); could return a null, but Convert.ToByte(o); returns 0.

If cmd.ExecuteScalar(); returns null its an error, because the value I'm looking for must be on database. If that value is not on database is an error.

What would you do here? Return a null or throw an exception?

VansFannel
  • 45,055
  • 107
  • 359
  • 626
  • This 'philosophy' question is better suited in programmers, not in stackoverflow. – kurast May 22 '14 at 12:49
  • It should throw an exception as cmd.ExecuteScalar() returning null depicts a break (uncaught while putting the record in the database) and should be raised when ever found. – Hemant Bhatt May 22 '14 at 12:49
  • It's not related to your question, but is passing in the connection string the best design? Seems like that should either be a property of the class (assuming it's some sort or repository or DAL), or part of the configuration (app.config or web.config). – D Stanley May 22 '14 at 12:54
  • @DStanley Yes, the connection string must in another place, but at this moment we don't have decided where. Thanks. – VansFannel May 22 '14 at 12:56

4 Answers4

8

You are pretty much answering your own question:

because the value I'm looking for must be on database. If that value is not on database is an error.

If your program doesn't function without that value you should throw an exception, if not you can return null and make the user of the library decide what to do next.

Jevgeni Geurtsen
  • 3,133
  • 4
  • 17
  • 35
1

I think if you want to do something if cmd.ExecuteScalar() returns null then you should return a null. But as you said

the value I'm looking for must be on database. If that value is not on database is an error.

then you should throw an exception type of InvalidOperationException rather than ArgumentNullException.

Mehul Vaghela
  • 478
  • 4
  • 20
0

If that value is not on database is an error.

An error in terms of "my belief system about the state of the system has been violated" or "the input must be invalid somehow"? It sounds like it's more the former - so I'd throw an exception. It sounds like the caller can't reasonably continue in this case. If they can, that's a different matter.

You might want to use InvalidOperationException, or perhaps create your own exception (InvalidDatabaseStateException for example) given that it's not really the state of this object which is invalid.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
0

According to here http://msdn.microsoft.com/en-us/library/ms229009(v=vs.110).aspx and here http://msdn.microsoft.com/en-us/library/ms229030(v=vs.110).aspx exceptions are very expensive and should be used with care.

I surely would use return Convert.ToByte(o) and test it on the calling function.

Marco Alves
  • 2,776
  • 3
  • 23
  • 33