HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Inventory check with too many else-ifs

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
elsewithinventorytooifsmanycheck

Problem

I am using if else statements:

public ActionResult Process(int? inventoryCheck, int? POCheck, int? PromotionCheck)
{
    //var result = db.ValidInventory(2);

    //1. When inventory is low:  
    if (inventoryCheck != null && POCheck == null && PromotionCheck == null)
    {
        db.ValidInventory(inventoryCheck);
    }
    //2. When PO is not generated 
    else if (POCheck != null && inventoryCheck == null && PromotionCheck == null)
    {
        db.ValidInventory(POCheck);
    }
    //3. For Promotion
    else if (POCheck == null && inventoryCheck == null && PromotionCheck != null)
    {
        db.ValidInventory(PromotionCheck);
    }
    else if (inventoryCheck != null && POCheck != null && PromotionCheck == null)
    {
        //4. When Inventory is low and PO is not generated 
        inventoryCheck = 4;
        db.ValidInventory(inventoryCheck);
    }
    else if (inventoryCheck != null && POCheck == null && PromotionCheck != null)
    {
        //5.When inventory is low and Promotion Check 
        inventoryCheck = 5;
        db.ValidInventory(inventoryCheck);
    }
    else if (inventoryCheck == null && POCheck != null && PromotionCheck != null)
    {
        //6. When PO is not generated and Promotion Check  
        inventoryCheck = 6;
        db.ValidInventory(inventoryCheck);
    }
    else if (inventoryCheck != null && POCheck != null && PromotionCheck != null)
    {
        //7. When PO is not generated and Promotion Check   and inventory is low 
        inventoryCheck = 6;
        db.ValidInventory(inventoryCheck);
    }

    return RedirectToAction("BackAnalysis");
}


Here more conditions are present, then I have to add more if else blocks.

How can I optimize it?

Solution

You should have to split your function absolutely.
You should have to group commons conditions statements and instructions.

However, I think there are deeper problem in your architecture which cause this piece of code. You have to rethink your problem at higher level.

For the three top conditions you can do this, for example :

public static void Test(int? inventoryCheck, int? POCheck, int? PromotionCheck)
    {
        bool bInventoryCheck = (inventoryCheck == null);
        bool bPOCheck = (POCheck == null);
        bool bPromotionCheck = (PromotionCheck == null);

        ValidInventory (!bInventoryCheck && bPOCheck && bPromotionCheck, inventoryCheck);
        ValidInventory (bInventoryCheck && !bPOCheck && bPromotionCheck, PromotionCheck);
        ValidInventory (bInventoryCheck && bPOCheck && !bPromotionCheck, POCheck);

    }

    public static void ValidInventory(bool validate, int? toValidate)
    {
        if (validate)
            //db.ValidInventory(toValidate);
    }


But I don't think it's a good solution because as you mention here, you have a lot of other cases. We can reduce your code by many way but it's bury the real problem.

In order to reduce we have to understand the concept behind the Process.

PS : Here the "else if" statement seems to be unnecessary, because you are testing every parameters in each case.

Code Snippets

public static void Test(int? inventoryCheck, int? POCheck, int? PromotionCheck)
    {
        bool bInventoryCheck = (inventoryCheck == null);
        bool bPOCheck = (POCheck == null);
        bool bPromotionCheck = (PromotionCheck == null);

        ValidInventory (!bInventoryCheck && bPOCheck && bPromotionCheck, inventoryCheck);
        ValidInventory (bInventoryCheck && !bPOCheck && bPromotionCheck, PromotionCheck);
        ValidInventory (bInventoryCheck && bPOCheck && !bPromotionCheck, POCheck);



    }

    public static void ValidInventory(bool validate, int? toValidate)
    {
        if (validate)
            //db.ValidInventory(toValidate);
    }

Context

StackExchange Code Review Q#52587, answer score: 2

Revisions (0)

No revisions yet.