current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 7 down vote favorite

I have code which works pretty well. I just need some of your opinions on how to write it better with fewer line of code. I want to use a ternary operator but I couldn't make it work so I did an if else instead.

Basically, I want to insert a price on my database. The price column is a nullable decimal. Even in my C# code, it's a nullable decimal as well.

if (items.Price == null)
{
    SqlParameter[] p =
    {
        new SqlParameter("@purchaseorder", items.PurchaseOrderItems),
        new SqlParameter("@modelnumber", items.ModelNumber), 
        new SqlParameter("@quantity", items.Quantity),
        new SqlParameter("@Price",  DBNull.Value),
        new SqlParameter("@Description", items.Description),
        new SqlParameter("@OrderNumber", items.OrderNumber),
        new SqlParameter("@Unit", items.Unit)
    };

    i = Dal.ExecuteNonQuery(sql,p);
}
else
{
    SqlParameter[] p =
    {
        new SqlParameter("@purchaseorder", items.PurchaseOrderItems),
        new SqlParameter("@modelnumber", items.ModelNumber), 
        new SqlParameter("@quantity", items.Quantity),
        new SqlParameter("@Price",  items.Price),
        new SqlParameter("@Description", items.Description),
        new SqlParameter("@OrderNumber", items.OrderNumber),
        new SqlParameter("@Unit", items.Unit)
    };

    i = Dal.ExecuteNonQuery(sql,p);
}
return i;

Can this be written without the else block?

share|improve this question
    
Why not make life easier and use an ORM? "if you’re writing ADO.Net code by hand, you’re stealing from your employer or client." – BCdotWEB 2 days ago
    
@BCdotWEB As a few coders out there commented on that post, if people followed that advice we wouldn't have all of those pretty ORMs out there. Sometimes, doing it the old, artisanal way is the best way to learn - we learn about the difficulties and discover what problems we need to solve. – Thales Pereira yesterday
up vote 6 down vote accepted

All you have to do is to use the ?: (ternary operator):

new SqlParameter("@Price", items.Price == null ? DBNull.Value : (object)items.Price)

Alternatively you could write an extension for it:

public static object PriceOrDefault(this ItemsType items)
{
    return items.Price == null ? DBNull.Value : (object)items.Price;
}

and use it instead:

new SqlParameter("@Price", items.PriceOrDefault())

As it turns out the Price is a decimal? so you may consider using the HasValue property and swap the values:

items.Price.HasValue ? (object)items.Price : DBNull.Value;

I find this looks better then the null-check.

share|improve this answer
    
Sorry, but i got a squiggly line underneath. s15.postimg.org/xmy26aduz/werw.png – Dikong Prigm 2 days ago
    
@DikongPrigm Just cust it to object. I edited the examples. SqlParameter excpects it anyway. – t3chb0t 2 days ago
    
That works.. Thank. I will wrap this to extension method. Thank you – Dikong Prigm 2 days ago
    
If you go with the helper method, I'd rather go with the more general object NulltoDBNull(object o) { return o != null ? o : DBNull.Value; }. – CodesInChaos yesterday
up vote 10 down vote

The most terse and declarative way to write this is to use C#'s ?? operator. Your line becomes:

new SqlParameter("@Price",  (object)items.Price ?? DbNull.Value),

The expression here will either return the non-null value of the left operand (items.Price), or return the operand to the right instead.

If you hadn't guessed, the ?? operator is basically syntactic sugar for if operl != null ? operl : operr.

share|improve this answer
    
nice, I actually know this one too but didn't think it would work like this with a nullable :-] – t3chb0t 2 days ago
2  
Using null coalescing is definitely the cleanest option. – William Mariager 2 days ago
up vote 4 down vote

This is down to personal taste, but if you do go down the extension method route suggested by t3chb0t, I'd consider making it generic so that you can reuse it for any future Nullable types you might use and you don't end up with a bunch of very similar looking functions for different fields. Something like this:

public static object DbValueOrNull<T>(this T? value) where T : struct
{
    return value.HasValue ? (object)value.Value : DBNull.Value;
}

Would allow:

new SqlParameter("@Price", items.Price.DbValueOrNull())
new SqlParameter("@SomeOtherNullableVar", items.SomeOtherNullableVar.DbValueOrNull())

etc.

share|improve this answer
    
wow, I believe I've never come up with the idea to use the nullable ? on a T + struct ;-) – t3chb0t 2 days ago
up vote 0 down vote

Have you taken a look at the System.Data.SqlTypes namespace - particularly SqlDecimal?

Building on some of the other answers, you can use SqlDecimal and the ?? operator as follows:

new SqlParameter("@Price",  items.Price ?? SqlDecimal.Null),
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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