3

For some context, the record class SaleAmount property is

public decimal? SaleAmount
{
    get;
    set;
}

Ideally I would go with this

record.SaleAmount.Value = 
sql.Reader.IsDBNull(IDX_SALEAMOUNT) ? null : 
    sql.Reader.GetDecimal(IDX_SALEAMOUNT);

Alas the compiler and this are not friends because...

Type of conditional expression cannot be determined because there is no implicit conversion between '' and 'decimal'

So how would you express this elegantly, and dont play the obvious card like below...

if (!sql.Reader.IsDBNull(IDX_SALEAMOUNT))
    record.SaleAmount = sql.Reader.GetDecimal(IDX_SALEAMOUNT);

Or is above the best solution?

10 Answers 10

14

If you're using C# 3, I'd actually make this easier for yourself using an extension method:

public static decimal? GetNullableDecimal(this DbReader reader, 
    int column)
{
    return reader.IsDBNull(column) ? (decimal?) null : 
        reader.GetDecimal(column);
}

Then your application code can just use:

record.SaleAmount = sql.Reader.GetNullableDecimal(IDX_SALEAMOUNT);
11
  • +1: Clever, readable function. In my opinion, it would even be worth defining such a static helper method in earlier C# versions.
    – Blixt
    Nov 10, 2009 at 20:20
  • 1
    Introducing static extension method complexity for a trivial problem seems like a bad tradeoff to me
    – Andomar
    Nov 10, 2009 at 20:25
  • I hate it that the rich keep getting richer and I really want to give someone other than Skeet the win on this...maybe someone should copy this answer and mod it just a tad so I can give them the check mark! Nov 10, 2009 at 20:26
  • 1
    As soon as this logic is involved in more than one place, having a method for it will definitely be worth it. The fact that it's an extension method doesn't change much, since that's just syntactic sugar and would very rarely cause any problems.
    – Blixt
    Nov 10, 2009 at 20:29
  • 1
    You guys are probably right. I'm a grumpy old programmer with a deep distrust of utility functions made in-house.
    – Andomar
    Nov 10, 2009 at 21:04
3

You could go with:

record.SaleAmount = 
    (sql.Reader.IsDBNull(IDX_SALEAMOUNT) ? null : 
     sql.Reader.GetDecimal(IDX_SALEAMOUNT)) as decimal?;

You could try another way:

// usage: new GenericDataReader(cm.ExecuteReader())
//        ...Get<decimal?>(...);
public class GenericDataReader : IDataReader
{
    // IDataReader implementation
    public T Get<T>(int ordinal)
    {
        if (_dataReader.IsDBNull(ordinal))
            return default(T);
        else
            return (T)_dataReader.GetValue(ordinal);
    }
}

More about this approach here.

2
  • This is a more "generic" :) approach to the problem, unfortunately this will lead to lot's of boxing/unboxing of value types, crippling performance unnecessarily (if used for intensive workloads). Nov 10, 2009 at 21:21
  • @Pop, I agree; but GetInt32(), for instance, probably do same thing internally Nov 11, 2009 at 11:17
3

I like to use the following generic extension method (that could be easily converted into a static helper function) to get possibly null values from a reader:

    public static T Get<T>(this IDataReader reader, string columnName)
    {
        if (reader[columnName] == DbNull.Value)
        {
            return default(T);
        }

        return (T)reader[columnName];
    }
7
  • that's going to box. Can get a noticable perf hit when use it everywhere. Nov 10, 2009 at 21:12
  • 1
    @Robert: To box is to encapsulate a value type inside an object, right? Can you explain why this function would box when called like Get<decimal?>(...) ?
    – Andomar
    Nov 10, 2009 at 21:38
  • @Robert: I'm not sure that I understand why you feel this will cause boxing. Unless I'm mistaken, neither implementation boxes and both have the same casting cost. According to Reflector, the SqlDataReader stores it's values in an instance of the SqlBuffer class. SqlBuffer has a property "Value" of type object in which it stores the database value by ordinal. When a typed version of the value is requested, SqlBuffer performs a cast to the appropriate type. SqlDataReader's GetDecimal requests the value from SqlBuffer resulting in the same cast operation that BlueSam is using. Nov 10, 2009 at 22:01
  • @Jesse, that is not what is happening. (And this is not just the case for SqlDatareader, but most implementations I have come across) The value is stored in a struct which can access most primitive types by reading them at the same field offset. GetInt32 will return this value as Int32. No boxing needed at all. Nov 11, 2009 at 11:22
  • 1
    @Jesse Check out SqlBuffer.get_Value line 7c or 124 in decimal: It will return the appropriate data field as object, thus boxing it. OTOH, any ValueType value in an object reference had to be boxed to get there, so there wasn't any research needed to get to this conclusion. ;-) Nov 11, 2009 at 14:17
2

You get that error because the type decimal (a value type) and null (a reference value) don't play together.

Also, since you're assigning to the Value property of the Nullable<decimal> type, you're effectively attempting to assign null to a decimal type which, again, is a no-go.

You can explicitly make the GetDecimal result a nullable decimal type by casting it:

record.SaleAmount = sql.Reader.IsDBNull(IDX_SALEAMOUNT) ? 
    null : (decimal?)sql.Reader.GetDecimal(IDX_SALEAMOUNT);

I would go with your second approach, though.

4
  • My only problem with approach two is it isnt totally evident to a JR programmer (people will have to maintain this after all); it actually takes a second to digest that we are only assigning when there is a value and just leaving it untouched otherwise. I know this seems trivial but trust me, people get confused over the simple stuff. Nov 10, 2009 at 20:12
  • You have to weigh this against using casts and a ternary conditional operator, which I find to be at least as confusing. Just write the above code explicitly then, or add a comment that non-assignment means null.
    – Blixt
    Nov 10, 2009 at 20:16
  • which makes me think that Skeet's idea of an extension method may make the most sense since it would be most evident and readable. But I refuse to give him the check mark, he has too many points! Nov 10, 2009 at 20:20
  • Well, there's a reason for that isn't there ;) Just go ahead =)
    – Blixt
    Nov 10, 2009 at 20:21
1

Firstly the Value property is readonly so you won't be able to assign to it anyway.

If you add a cast to the false part of the conditional to cast to nullable decimal the compiler should be happy. Try the following

record.SaleAmount = sql.Reader.IsDBNull(IDX_SALEAMOUNT) ? 
    null : (decimal?) sql.Reader.GetDecimal(IDX_SALEAMOUNT);
0

Why not use the null coalescing operator?

record.SaleAmount = (decimal?)sql.Reader.GetDecimal(IDX_SALEAMOUNT)
    ?? null;
3
  • 2
    GetDecimal returns decimal, which can never be null, thus ?? won't work.
    – Blixt
    Nov 10, 2009 at 20:13
  • I see you updated your post, but now it's worse. ?? null is logically wrong (return value if it's not null and return null if it is null, which it never will be since decimal can't be null)
    – Blixt
    Nov 10, 2009 at 20:14
  • msdn.microsoft.com/en-us/library/… See the remarks section, it should throw an InvalidCastException if it is not actually a decimal Nov 10, 2009 at 20:15
0

To me, the cleanest solution is to remove it from the business logic tier altogether and just make the query return a non-null value to the reader in the first place

For example if using SQL Server..

SELECT IsNull(SalesAmount,0) ...
3
  • 1
    This depends wholly on what NULL and 0 actually mean! There's an important difference between "there is no sales amount available" and "the sales amount is 0.0"
    – Blixt
    Nov 10, 2009 at 20:22
  • But that would return 0; he wants null. Not SqlDecimal.Null, but null :)
    – Andomar
    Nov 10, 2009 at 20:23
  • I agree it would make for cleaner code but the others are right, someone can sell for zero and null means it hasnt sold. Nov 10, 2009 at 20:29
0

I use a FieldHelper class with Nullable<T> ToXXX(object) methods. Here's the Decimal example (this was written during pre-Linq days - about 2-3 years ago, you're welcome to replace the delegate construct to Linq):

/// <summary>
/// Gets a nullable value.
/// </summary>
/// <param name="aValue">The value to be converted.</param>
/// <returns>The converted value.</returns>
public static Nullable<decimal> ToDecimal(object aValue)
{
   return ToNullable<decimal>(aValue,
     delegate(object aConvertableValue)
     {
        return Convert.ToDecimal(aConvertableValue);
     });
}

/// <summary>
/// Converts the given value if necessary.
/// </summary>
/// <param name="aValue">A value from the database.</param>
/// <param name="aConversion">A conversion delegate.</param>
/// <returns>null, or the given value.</returns>
private static Nullable<T> ToNullable<T>(
        object aValue, Converter<T> aConverter) where T : struct
{
  if (aValue == DBNull.Value || aValue == null)
     return null;
  else if (aValue is T)
     return (T)aValue;
  else
     return aConverter(aValue);
}
0

To get this this without paying too much in terms of runtime overhead, you can't be too picky, I'm afraid.

However, there's a more concise syntax:

public static Nullable<T> GetNullableValue<T>(this IDataRecord record, 
    int columnIndex, Func<int, T> getValue)
  where T: struct;
{
  if (record.IsDbNull(columnIndex))
    return null;
  else
    return getValue(columnIndex);
}

var xyz = reader.GetNullableValue(0, reader.GetDecimal);
0

IsDbNull(int) is usually much slower that using methods like GetSqlInt32 and then comparing to DBNull.Value or using it's own .IsNull Like:

    public static int Int32(this SqlDataReader r, int ord)
    {
        var t = r.GetSqlInt32(ord);
        return t.IsNull ? default(int) : t.Value;
    }

Tried a few template solutions but to no avail so far. The problem is that all Sql-types (SqlInt32 here) types are actually structs and while they all have .Value property C# doesn't have real templates to handle that. Also they have their own INullable interface which has only .IsNull and is not conpatible with Nyllable<>.

I suspect that one would need full set of Sql-types as C# templates or to add ICOnvertible to them in order to be able to have just one or two templated methods.

If someone has maybe an idea with a functional trick or two speak up :-)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.