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 3 down vote favorite

In this Stored procedure (called by this inventory-checking function), 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
up vote 3 down vote

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
    
... stored procedures are NOT necessarily faster, especially when they are doing different operations depending on a parameter as this makes it hard for the optimizer to optimize. I also do not know what you mean by 'backend process'. I would agree that this should definitely be split into 7 different queries, whether they are stored procedures or views – Milney Aug 3 at 12:49

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.