Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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
In this Stored procedure (called by [this inventory-checking function](http://codereview.stackexchange.com/q/52587/9357)), many if else if statement are used.  How can it be improved?

          IF ( @count = 1 ) 
         (SELECT skumaster.sku AS SKU, 
                         ( skumaster.minimumstock - Count(*) ) AS ReorderQuantity, 
                         'LowInventory'                        AS Description 
                  FROM   skumaster skuMaster 
                         JOIN inventorymaster inventoryMaster 
                           ON skumaster.sku = inventorymaster.sku 
                  GROUP  BY skumaster.sku, 
                            skumaster.minimumstock, 
                            skumaster.name 
                  HAVING Count(*) < skumaster.minimumstock) 
         ELSE IF( @count = 2 ) 
         (SELECT weeklyorderlist.sku AS SKU, 
                         weeklyorderlist.quantity AS ReorderQuantity, 
                         'NoPO'                   AS Description 
                  FROM   weeklyorderlist 
                  WHERE  weeklyorderlist.pocgen = 'true') 
           ELSE IF( @count = 3 ) 
         (SELECT promotionmaster.sku AS SKU, 
                         ( skumaster.minimumstock + skumaster.maximumstock / 2 ) - 
                         flatdiscount.quantityfordiscount AS ReorderQuantity, 
                         'Flat Discount'                  AS descp 
                  FROM   promotionmaster 
                         JOIN flatdiscount 
                           ON promotionmaster.promotiontypeid = 
                              flatdiscount.promotiontypeid 
                         JOIN skumaster 
                           ON promotionmaster.sku = skumaster.sku 
                  WHERE  promotionmaster.enddate > currenttimezone 
                  --Select from Variable discount  
                  UNION 
                  SELECT variablediscount.sku AS SKU, 
                         variablediscount.quantity AS ReorderQuantity, 
                         'Variable Discount'       AS descp 
                  FROM   promotionmaster 
                         JOIN variablediscount 
                           ON promotionmaster.promotiontypeid = 
                              variablediscount.promotiontypeid 
                  WHERE  promotionmaster.enddate > currenttimezone) 
           ELSE IF( @count = 4 ) 
        (SELECT skumaster.sku AS SKU, 
                         ( skumaster.minimumstock - Count(*) ) AS ReorderQuantity, 
                         'LowInventory'                        AS Description 
                  FROM   skumaster skuMaster 
                         JOIN inventorymaster inventoryMaster 
                           ON skumaster.sku = inventorymaster.sku 
                  GROUP  BY skumaster.sku, 
                            skumaster.minimumstock, 
                            skumaster.name 
                  HAVING Count(*) < skumaster.minimumstock 
                  UNION 
                  SELECT weeklyorderlist.sku AS SKU, 
                         weeklyorderlist.quantity AS ReorderQuantity, 
                         'NoPO'                   AS Description 
                  FROM   weeklyorderlist 
                  WHERE  weeklyorderlist.pocgen = 'true') 
        union 
        (SELECT skumaster.sku AS SKU, 
                         ( skumaster.minimumstock - Count(*) ) AS ReorderQuantity, 
                         'LowInventory'                        AS Description 
                  FROM   skumaster skuMaster 
                         JOIN inventorymaster inventoryMaster 
                           ON skumaster.sku = inventorymaster.sku 
                  GROUP  BY skumaster.sku, 
                            skumaster.minimumstock, 
                            skumaster.name 
                  HAVING Count(*) < skumaster.minimumstock 
                  UNION 
                  SELECT promotionmaster.sku AS SKU, 
                         ( skumaster.minimumstock + skumaster.maximumstock / 2 ) - 
                         flatdiscount.quantityfordiscount AS ReorderQuantity, 
                         'Flat Discount'                  AS descp 
                  FROM   promotionmaster 
                         JOIN flatdiscount 
                           ON promotionmaster.promotiontypeid = 
                              flatdiscount.promotiontypeid 
                         JOIN skumaster 
                           ON promotionmaster.sku = skumaster.sku 
                  WHERE  promotionmaster.enddate > currenttimezone 
                  --Select from Variable discount  
                  UNION 
                  SELECT variablediscount.sku AS SKU, 
                         variablediscount.quantity AS ReorderQuantity, 
                         'Variable Discount'       AS descp 
                  FROM   promotionmaster 
                         JOIN variablediscount 
                           ON promotionmaster.promotiontypeid = 
                              variablediscount.promotiontypeid 
                  WHERE  promotionmaster.enddate > currenttimezone) 
           ELSE IF( @count = 5 ) 
        (SELECT skumaster.sku AS SKU, 
                         ( skumaster.minimumstock - Count(*) ) AS ReorderQuantity, 
                         'LowInventory'                        AS Description 
                  FROM   skumaster skuMaster 
                         JOIN inventorymaster inventoryMaster 
                           ON skumaster.sku = inventorymaster.sku 
                  GROUP  BY skumaster.sku, 
                            skumaster.minimumstock, 
                            skumaster.name 
                  HAVING Count(*) < skumaster.minimumstock 
                  UNION 
                  SELECT promotionmaster.sku AS SKU, 
                         ( skumaster.minimumstock + skumaster.maximumstock / 2 ) - 
                         flatdiscount.quantityfordiscount AS ReorderQuantity, 
                         'Flat Discount'                  AS descp 
                  FROM   promotionmaster 
                         JOIN flatdiscount 
                           ON promotionmaster.promotiontypeid = 
                              flatdiscount.promotiontypeid 
                         JOIN skumaster 
                           ON promotionmaster.sku = skumaster.sku 
                  WHERE  promotionmaster.enddate > currenttimezone 
                  --Select from Variable discount  
                  UNION 
                  SELECT variablediscount.sku AS SKU, 
                         variablediscount.quantity AS ReorderQuantity, 
                         'Variable Discount'       AS descp 
                  FROM   promotionmaster 
                         JOIN variablediscount 
                           ON promotionmaster.promotiontypeid = 
                              variablediscount.promotiontypeid 
                  WHERE  promotionmaster.enddate > currenttimezone) z


           ELSE IF( @count = 6 ) 
       --Select from WeeklyOrder 
                 (SELECT weeklyorderlist.sku AS SKU, 
                         weeklyorderlist.quantity AS ReorderQuantity, 
                         'NoPO'                   AS Description 
                  FROM   weeklyorderlist 
                  WHERE  weeklyorderlist.pocgen = 'true' 
                  UNION 
                  SELECT promotionmaster.sku AS SKU, 
                         ( skumaster.minimumstock + skumaster.maximumstock / 2 ) - 
                         flatdiscount.quantityfordiscount AS ReorderQuantity, 
                         'Flat Discount'                  AS descp 
                  FROM   promotionmaster 
                         JOIN flatdiscount 
                           ON promotionmaster.promotiontypeid = 
                              flatdiscount.promotiontypeid 
                         JOIN skumaster 
                           ON promotionmaster.sku = skumaster.sku 
                  WHERE  promotionmaster.enddate > currenttimezone 
                  --Select from Variable discount  
                  UNION 
                  SELECT variablediscount.sku AS SKU, 
                         variablediscount.quantity AS ReorderQuantity, 
                         'Variable Discount'       AS descp 
                  FROM   promotionmaster 
                         JOIN variablediscount 
                           ON promotionmaster.promotiontypeid = 
                              variablediscount.promotiontypeid 
                  WHERE  promotionmaster.enddate > currenttimezone) 

           ELSE IF( @count = 7 ) 
        --Indicate When Inventory Check and PO Check and Promotion Check is checked 

                 --Select the union from all Inventory Check and PO Check and Promotion Check 
                 (SELECT skumaster.sku AS SKU, 
                         ( skumaster.minimumstock - Count(*) ) AS ReorderQuantity, 
                         'LowInventory'                        AS descp 
                  FROM   skumaster skuMaster 
                         JOIN inventorymaster inventoryMaster 
                           ON skumaster.sku = inventorymaster.sku 
                  GROUP  BY skumaster.sku, 
                            skumaster.minimumstock, 
                            skumaster.name 
                  HAVING Count(*) < skumaster.minimumstock 
                  UNION 
                  SELECT weeklyorderlist.sku AS SKU, 
                         weeklyorderlist.quantity AS ReorderQuantity, 
                         'NoPO'                   AS descp 
                  FROM   weeklyorderlist 
                  WHERE  weeklyorderlist.pocgen = 'true' 
                  UNION 
                  SELECT promotionmaster.sku AS SKU, 
                         ( skumaster.minimumstock + skumaster.maximumstock / 2 ) - 
                         flatdiscount.quantityfordiscount AS ReorderQuantity, 
                         'Flat Discount'                  AS descp 
                  FROM   promotionmaster 
                         JOIN flatdiscount 
                           ON promotionmaster.promotiontypeid = 
                              flatdiscount.promotiontypeid 
                         JOIN skumaster 
                           ON promotionmaster.sku = skumaster.sku 
                  WHERE  promotionmaster.enddate > currenttimezone 
                  UNION 
                  SELECT variablediscount.sku AS SKU, 
                         variablediscount.quantity AS ReorderQuantity, 
                         'Variable Discount'       AS descp 
                  FROM   promotionmaster 
                         JOIN variablediscount 
                           ON promotionmaster.promotiontypeid = 
                              variablediscount.promotiontypeid 
                  WHERE  promotionmaster.enddate > currenttimezone)
share|improve this question
    
The last case (ELSE IF (@count = 7)) seems incomplete. – 200_success Jun 6 '14 at 8:30

I see seven independent queries. There is no reason to put them all in one stored procedure that dispatches based on the @count parameter. (Furthermore, it shouldn't be named "count". "Mode" would be more appropriate.)

Once you treat them as independent queries, there is no need for a stored procedure anymore. You could create seven VIEWs instead. The views that involve a UNION could even reference other views, to reduce code duplication.

share|improve this answer
    
Since stored Procedure works faster and it is purely an backend process i thought given in stored procedure will help but in future if more condition are to be checked then it will increase the code unnecessary. And only three query are independent other are union based on condition – Priya Jun 6 '14 at 8:31

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.