Is this SP using best practices ......can anyone explain what this SP is doing.

Can someone tell me what would a person do in the event that a user used best practices so that this SP code made a bit more sense. It would appear that this SP is okay because it returns results in the query. However to me it seems like it is missing something. It seems like the code is not balanced in a way that records return normally. I would not know as I am still learning about SPs.  I am especially confused about the temp tables and the middle section after the temp1 table is populated and what happens between the to selects that has the Left Outer Join in it. Can someone explain to me what this SP is doing exactly. Another thing is I do not understand why it does not fail when executed when there are selects statements with 6 fields and others with only 4....?

/****** Object:  StoredProcedure [dbo].[Rpt_Route_Depositby_Date_2]    Script Date: 10/08/2013 08:51:24 ******/

ALTER Procedure [dbo].[Rpt_Route_Depositby_Date_2]
		@FirstDeliveryDate as Datetime, 
		@LastDeliveryDate as Datetime

--     First Select Putting Data into Temp Table 4

into #temp4 
from dbo.ROSS_ROUTE r
--     Second Select Putting Data into Temp Table 1

		sh.OH_DELIVERY_DATE as PostDate, 
		isnull(sum(sh.OH_TICKET_TOTAL),0) as InvoiceAmt,
		isnull(sum (ROA.ROA_AMT),0) as ROA_AMT,
		isnull(sum(de.AMOUNT),0) as Expenses  
into #temp1
from dbo.ROSS_SALESHDR sh 
		left outer join dbo.ROSS_DRIVER_EXPENSES de on sh.OH_ROUTE_NUMBER = de.ROUTE_NUMBER

left outer join 

		sh.OH_DELIVERY_DATE as PostDate,
from dbo.ROSS_SALESHDR sh 
where sh.OH_ACCT_AR_TYPE = '000001' and  sh.OH_CUSTOMER_TYPE_ID In ('000003','000006')
where sh.OH_ACCT_AR_TYPE = '000001' and  sh.OH_CUSTOMER_TYPE_ID = '000003'
		and sh.OH_DELIVERY_DATE between @FirstDeliveryDate  and @LastDeliveryDate 
group by 

isnull(sum(SH_DEPOSIT),0) as Deposit
into #temp2
group by rsh.SH_ROUTE, rsh.SH_ROUTE_DATE, rsh.SH_SETTLED 

into #temp3 
from #temp1 t1 left outer join #temp2 t2 on 
		t1.OH_ROUTE_NUMBER = t2.SH_ROUTE and t1.PostDate = t2.SH_ROUTE_DATE
order by 1,2,4

/* Join distinct route table */
from #temp3 t3 inner join #temp4 r

drop table #temp1;
drop table #temp2;
drop table #temp3;
drop table #temp4;
--select * from #temp1 

--exec [dbo].[Rpt_Route_Depositby_Date_2] '2013-01-14 00:00:00.000', '2013-3-14 00:00:00.000'

Open in new window

RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Asked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Jim HornMicrosoft SQL Server Developer, Architect, and AuthorCommented:
For starters, the developer that authored this should be flogged for creating an object without any meaningful code comments or version control history.

Guessing this is a SP that is the data source for a report named something like 'Route_Depositby_Date_2'

>I am especially confused about the temp tables
Essentially it's a temp table in memory, that only lives while the object that creates it is executing.  Very handy for long SP's.

>First Select Putting Data into Temp Table 4
Yeah, that's real helpful to explain what this code does.  Not.
Objects should be given some meaningful name such as #customers, not #tmp1, #tmp2, etc.

INSERTING into a temp table that has not been created yet.
Better to CREATE TABLE #tmp first.

>what happens between the to selects that has the Left Outer Join in it.
Not sure what you mean here.

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Author Commented:
what happens between the to selects that has the Left Outer Join in it.
Not sure what you mean here. ....

Well I was trying to make sense that the creator of this used a consistent Insert into Temp1 or Temp2 or ...3, 4. Etc.
I was not sure why the Left Outer Join in the Selects where the second select did not have a Temp? in it. To me it was confusing as to what the second section was. Not to mention it seems longer than the other Select statements.
RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Author Commented:
The reason I ask is ...... what if I need to put another field in this SP. I need to put in the field for the Invoice is called OH_TICKET-NUMBER. Where would I put that how many times do I need to drop it in the SP here. Because there was no consitency to start with I have little to go on as a good example. Hence why I thought I need a best practices approach to this.
The Ultimate Tool Kit for Technolgy Solution Provi

Broken down into practical pointers and step-by-step instructions, the IT Service Excellence Tool Kit delivers expert advice for technology solution providers. Get your free copy for valuable how-to assets including sample agreements, checklists, flowcharts, and more!

Jim HornMicrosoft SQL Server Developer, Architect, and AuthorCommented:
>I was not sure why the Left Outer Join in the Selects where the second select did not have a Temp?
LEFT OUTER JOIN = left join, meaning return all the rows from the table referenced LEFT of the JOIN, populated with any values that may match from the RIGHT of the JOIN.

Looking at the pile of goo code I'm not going to hazard a guess.

>I need to put in the field for the Invoice is called OH_TICKET-NUMBER.
Probably the only way to answer that is to ask what table is it in, and where in the SP does it reference that table.  You'll likely add it there.
Good morning Ruavol2,

          While I can't exactly tell you the specifics of what this is doing without knowing the structure of the database, the basics of what it LOOKS like it is doing are :

1. Looking at your route master table and populating a temp table with the result set of that table (temp #4)
2. then it LOOKS like another temp table (temp #1) is being populated from:
        (a)  the Ross Sales Header table (with no filters), it's pulling in columns like the delivery date, invoice total, discounts etc.,
        (b) the Ross Sales Header table WITH filters, only returning Accounts Receivable Type 00001 or Customer Type ID 00003 or 00006 - it also is filtering out any sales headers where cash wasn't received (or looks like it)  that table is aliased as "ROA" and it is then joined back to the FULL sales header table.
3.  Temp # 2 is created from a table The Route Settlement Header table, which I have no idea without seeing it what it is for, but my guess is that this table should have all the routes that have been ... fully paid? that's my thought, then that table is tied to Temp 3 on what looks like primary keys and route dates

End result joins Temp 3, Temp 4 and Temp 2 together - you should be getting:

100% of the Ross Sales Header Table,
Values in Columns Post Date and ROA_AMT for the Ross Sales Header orders that were delivered between the First and Last date you entered as the parameters for the SP
and a column for the total deposits paid on the orders in the column Deposit along w/ the description of the route.  

That's all I can tell you and again this is only my assumption without knowing the tables.  HOWEVER, I will say you have multiple examples of code NOT using best practices:

1.  First would be the lack of column aliases in the derived tables
2.  If Account Type and Customer ID Type are integers, they shouldn't be encapsulated in single quotes, I believe SQL does implicit conversion so it's not a big deal, but it still costs in performance albeit slightly
3. ORDER BY statement, though it CAN allow for ordinal position (e.g. Order By 1, 2, 3) should never be used with ordinal position but should always reference column names or aliases  as in Order by OH_Route_Number, PostDate, InvoiceAmt
4. Never use SELECT *, ... ever.  As in t3.*  -- it's just lazy and not best practices

Beyond that, here's a few other things to consider:
(1) why select into temp tables at all if you don't intend to manipulate the data?  Just select the values from the base tables
(2) why filter on OH_Customer_Type_ID IN ('00003', '00006') and then on the outer query only filter on OH_Customer_Type_ID = '00003' it would be easier to just do the = in the inner query it would seem.
(3) why not name the temp tables in the order of creation? (Temp1 is first, 2 second etc.,)? makes the logic easier to read.

That's all I have, sorry for the long response
RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Author Commented:
Jim -  The table that it comes from is the ROSS_SALESHDR sh
Table. i was trying to figure out how to add that. When I do I get the Aggregate Group by error. When I put it in a group I go from 4200 rows to 260000 rows. With lots of duplicates.

I did not know whether I needed to do a select distinct on all of the Select Statements or a SELECT.

why does the Second have of the query "after the Left Outer Join" section have the same number of fields as the SELECT section above it.
RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Author Commented:
Excellent description CoreConcepts. Wow....!
Jim HornMicrosoft SQL Server Developer, Architect, and AuthorCommented:
ruavol2 - I think you owe CoreConcepts a case of beer for that one.   Having a question like 'tell me everything that's wrong with this pile of code' is arguably above the scope of a typical EE question, and that's a lot of free analysis.
Ha - thank you jim, much appreciated.
I'm impressed too coreconcepts - nice job.

an another from Jim for quoting: "goo code", do we need that as a topic?
RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Author Commented:
I keep cases in my fridge in the mountains of the Appalachia for just such occasions. I have actually tried to send beverages to EE members like mlmcc who has become a personal friend whom I speak to about work and work with almost nightly on projects all over. I am from the state that produces the most popular whiskey in the world. Goes by the name of Mr. Daniels.

So I concurr Dr. Horn I concurr.

On another note the code is a bit messy and has been difficult to cipher. It seemed to be messy and incomplete and I am a person who works best in breaking it all down with descriptors and easy to read (with spaces and seperators) type stuff. If it is a big blob then I think my eyes and mind just do not seperate things well.

CoreConcepts.....that was an impressive display of work....PERIOD. So thank you both
RUA Volunteer2?Tableau Trainer & Consultant Sales Exec.Author Commented:
Well now someone might gleen some best practices and some examples of what Good Code looks like. Now if there was a place I could find that actually set those standards so I can follow them.
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Microsoft SQL Server 2008

From novice to tech pro — start learning today.