CODEWITHSUNDEEP
c#asp.net-mvc
John
John

Reputation: 3945

how to refactor if statements into one statement

    PROJ_ClientAccount Client = db.PROJ_ClientAccount.Where(x => x.Id == 1).FirstOrDefault();
    PROJ__VATRateRecord VatRecord = db.PROJ_VATRateRecord.Where(x => x.Id == 2).FirstOrDefault();
    PROJ__ProductRecord ProductRecord = db.PROJ_ProductRecord.Where(x => x.Id == sale.Value.ProductId).FirstOrDefault();

if (Client == null)
{
throw new Exception("Error creating new Order Record, Client Account can't be empty");
}

if (VatRecord == null)
{
throw new Exception("Error creating new Order Record, VAT can't be empty");
}

if (ProductRecord == null)
{
throw new Exception("Error creating new Order Line Record, ProductRecord can't be empty");
}

I would like to refactor this and only use one if statement. If ("any of record is null") { throw new exception("rror creating order, "record" cant be empty }

thanks

Upvotes: 0

Views: 115

Answers (4)

senyor
senyor

Reputation: 352

It is better to leave three statements as is but make fall back earlier.

So move each statement just right after each operation like this:

PROJ_ClientAccount Client = db.PROJ_ClientAccount.Where(x => x.Id == 1).FirstOrDefault();

if (Client == null)
{
throw new Exception("Error creating new Order Record, Client Account can't be empty");
}

PROJ__VATRateRecord VatRecord = db.PROJ_VATRateRecord.Where(x => x.Id == 2).FirstOrDefault();

if (VatRecord == null)
{
throw new Exception("Error creating new Order Record, VAT can't be empty");
}

PROJ__ProductRecord ProductRecord = db.PROJ_ProductRecord.Where(x => x.Id == sale.Value.ProductId).FirstOrDefault();

if (ProductRecord == null)
{
throw new Exception("Error creating new Order Line Record, ProductRecord can't be empty");
}

Upvotes: 0

rene
rene

Reputation: 42494

I don't think throwing exceptions for businessrule validation is the best idea but if you have to stick closely to what you have you could make use of an extension method for FirstOrDefault with a custom exception message as parameter.

This is the extension method:

public static class BusinessValidator
{
    public static TSource FirstOrDefault<TSource>(
        this IEnumerable<TSource> source, string message)
    {
        TSource src = source.FirstOrDefault();
        if (src == null)
        {
            throw new Exception(message);
        }
        return src;
    }
}

Your current code wil be refactored to:

 PROJ_ClientAccount Client = db
                   .PROJ_ClientAccount
                   .Where(x => x.Id == 1)
                   .FirstOrDefault("Error creating new Order Record, Client Account can't be empty");
PROJ__VATRateRecord VatRecord = db
                   .PROJ_VATRateRecord
                   .Where(x => x.Id == 2)
                   .FirstOrDefault("Error creating new Order Record, VAT can't be empty");
PROJ__ProductRecord ProductRecord = db
                   .PROJ_ProductRecord
                   .Where(x => x.Id == sale.Value.ProductId)
                   .FirstOrDefault("Error creating new Order Line Record, ProductRecord can't be empty");

Upvotes: 0

Sudhakar Tillapudi
Sudhakar Tillapudi

Reputation: 26219

as i suggested you in my comments this is how you achieve:

if (Client == null || VatRecord == null || ProductRecord == null)
{
   throw new Exception("Error creating new Order,Record can't be empty");
}

but if all of the types(being compared) are similar then you can use null-coalescing operator as below:

Note: below example works only if all of the three types are similar.

var obj = Client ?? VatRecord ?? ProductRecord ?? null;
if(obj == null)
{
throw new Exception("Error creating new OrderRecord can't be empty");
}

Upvotes: 0

Blue Ice
Blue Ice

Reputation: 7930

if (Client == null || VatRecord == null || ProductRecord == null)
{
    throw new Exception("Error creating new Order, \"record\" cannot be empty");
}

This creates an or statement between the 3 conditions. If any one of them occurs, then the exception will be thrown.

However, this refactoring doesn't make much sense to me. It is probably a better idea to leave the code as you had it, to provide more descriptive error messages for the user.

Upvotes: 3

Related Questions