May 13, 2008 at 6:27 am
Here is a block of SQL code
I'm working on. I have a feeling it might be changed to
a bunch of UPDATE statements without any cursors or loops.
Plus I would like to review the logic how they
prepare @tot_items and @ship_items parameters for the final formula:
set @mfg_ship = (@shipcost * (@ship_items/@tot_items))
update usfpims.dbo.box set shipcost = @mfg_ship where boxid = @boxid
It's a bit messy. I want to see if it can be more straight forward, easy to read and understand.
Here is the code:
use USFRET
--Update_shipcost
Declare @boxid char(10), @shipcost money, @mfg_ship money, @tot_items money, @ship_items money,
@Province varchar(2), @Pharmacy varchar(3)
set @shipcost=0.0
set @mfg_ship=0.0
set @tot_items=0
set @ship_items=0
Declare ship_cost_cur SCROLL Cursor for
select distinct b.boxid, b.shipcost, d.province, Pharmacy =
Casewhen left(d.store_num,3) = 'ZEL' then 'ZEL'
when left(d.store_num,3) = 'HBC' then 'ZEL'
when d.sdm_store = 'Y' then 'SDM'
Else 'OTH'
End
from box b
inner join dispenser d on b.dispenserid = d.dispenserid
inner join scan s on b.boxid = s.boxid
where (s.credit is null and b.status = 'OUT' and b.boxid not in (select boxid from usfpims.dbo.box) ) and
((s.client = 'GSK' and d.province in ('QC','SK') and d.sdm_store = 'Y')
or
(s.client = 'GSK' and d.province in ('QC','SK','PE','NS','NB','NF') and ( left(d.store_num,3) in ('ZEL','HBC') ) )
or
((s.client = 'MCP' or s.client = 'JNJ') and d.sdm_store = 'Y')
or
(s.client = 'PCP' or s.client = 'PCR' or s.client = 'HOS')
or
((s.client in (select clientcode from manu_clients)) and (left(d.store_num,3) not in ('ZEL','HBC') and d.sdm_store != 'Y') ))
Open ship_cost_cur
Fetch next from ship_cost_cur into @boxid, @shipcost, @Province, @Pharmacy
While @@FETCH_STATUS = 0
BEGIN
set @tot_items = (select count(*) from scan where boxid = @boxid)
if @Province not in ('QC', 'SK') and @Pharmacy = 'SDM'
Begin
select @ship_items = count(*) from scan where boxid = @boxid and
(((client in ('MCP','JNJ','PCP','PCR','HOS')) and
(client in (select clientcode from policy where deduct_ship = 'N') or (reason = 'RCL'))))
End
if @Province in ('QC', 'SK') and @Pharmacy = 'SDM'
Begin
select @ship_items = count(*) from scan where boxid = @boxid and
(((client in ('GSK','MCP','JNJ','PCP','PCR','HOS')) and
(client in (select clientcode from policy where deduct_ship = 'N') or (reason = 'RCL'))))
End
if @Province in ('QC','SK','PE','NS','NB','NF') and @Pharmacy = 'ZEL'
Begin
select @ship_items = count(*) from scan where boxid = @boxid and
(((client in ('GSK','PCP','PCR','HOS')) and
(client in (select clientcode from policy where deduct_ship = 'N') or (reason = 'RCL'))))
End
if @Province not in ('QC','SK','PE','NS','NB','NF') and @Pharmacy = 'ZEL'
Begin
select @ship_items = count(*) from scan where boxid = @boxid and
(((client in ('PCP','PCR','HOS')) and
(client in (select clientcode from policy where deduct_ship = 'N') or (reason = 'RCL'))))
End
if @Pharmacy = 'OTH'
Begin
select @ship_items = count(*) from scan where boxid = @boxid and
(((client in (select clientcode from manu_clients)) and
(client in (select clientcode from policy where deduct_ship = 'N') or (reason = 'RCL'))))
End
If @tot_items <> 0
Begin
set @mfg_ship = (@shipcost * (@ship_items/@tot_items))
update usfpims.dbo.box set shipcost = @mfg_ship where boxid = @boxid
End
set @shipcost=0.0
set @mfg_ship=0.0
set @tot_items=0
set @ship_items=0
Fetch next from ship_cost_cur into @boxid, @shipcost, @Province, @Pharmacy
END
deallocate ship_cost_cur
May 13, 2008 at 6:59 am
You are right it is messy. There is a lot going on here, can you provide a brief explanation of the business rules for the code? I also have a question about this line:
(s.credit is null and b.status = 'OUT' and b.boxid not in (select boxid from usfpims.dbo.box) )
Are all the tables in usfpims? If this is true, then the code above should never return "true" since you can't be in box (in the from clause) and not in box (where clause).
Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question
May 13, 2008 at 10:03 am
Jack,
SQL is being executed in "USFRET" database.
I updated the original posting.
May 13, 2008 at 10:18 am
How about the "rules" for this process? I think it is way too complex to try to figure out all the rules from the code.
You may want to break it down into smaller chunks, possibly based on all the "or's" in the where clause of the cursor.
Jack Corbett
Consultant - Straight Path Solutions
Check out these links on how to get faster and more accurate answers:
Forum Etiquette: How to post data/code on a forum to get the best help
Need an Answer? Actually, No ... You Need a Question
May 13, 2008 at 11:37 am
That's the challenge.
Nobody knows the rules.
In the last 4 years the company had
a bunch of junior programmers who
coded it in different ways.
Now the company is asking me if I can
figure out the logic from the code, analyze it, verify with the operations manager,
and if possible to re-write it in a more readable, logical way.
Right away i can see there's too much hard coding.
Calculation of shipment cost should not be done based on
each client (... WHERE client IN ('PCR',CRP',...)
I need to find the pattern, re-organize the criteria a bit.
I might need to change the schema of "box" or "scan" or "policy" tables.
To me "shipcost" should be calculated based on client "shipment cost policies/preferences".
Something like
If client.ship_deduct = 'N' and client.ship_charges_rule_1 = 'Yes'
update box set shipcost = @shipcost * @shipcharges
I don't want to see any
If @Province IN ('QC,'NS') and Pharmacy = 'SDM'
set @ship_items = count(*) from ....
where client IN ('PCR,'CPR'..)
My goal to get rid of hard-coded stuff.
Business rules are not clearly defined, not documented.
I have to clarify all of that.
Viewing 5 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply