patterncsharpMinor
Inventory check with too many else-ifs
Viewed 0 times
elsewithinventorytooifsmanycheck
Problem
I am using
Here more conditions are present, then I have to add more
How can I optimize it?
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 :
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.
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.