Home » SQL & PL/SQL » SQL & PL/SQL » Is this statement repeating itself (code review) or can the last 2 be combined
Is this statement repeating itself (code review) or can the last 2 be combined [message #435534] Wed, 16 December 2009 10:21 Go to next message
sjackson1972
Messages: 13
Registered: December 2009
Junior Member
Global Variable
year varchar2(4)
ssn number(10)
name varchar2(50)
error boolean
cmd varchar2(500)
step number(10)
id_count number(10)
tsn number(10)
last_ids varchar2(20)

Procedure School (test1 in varchar2:= 'none'
                   test1 in varchar2:= 'all',
                   auto in varchar2:= 'n');
 
if auto="n" then
    if (test1 is null or test1='none') and 
       (test2 is null or test 2 ='all') then 
       score :=A;
       dbms_output.put_line('open book1');
       open book1;
elsif (test1 is not null and test1 =! 'none) then
       score:=B;
       this_test1 := test1 || %;
       dbms_output.put_line('open book2');
       open book2;
elsif (test2 is not null and test2 != all) then
       score:=C;
       this_test2 := test2 || %;
       dbms_output.put_line('open book3');
       open book3;
  endif;
else
          // I think the last 2 are repeating or can be combined
    [b]if score = A then
         dbms_output.put_line(open book1);
         open book1;
    elsif score = B then
         this_test1 := test1 || %;
         dbms_output.put_lineoutput(open book2) 
         open book2;
   elsif score = c then
        dbms_output.put_line(Open book3)
        open book3;
 endif;
endif;
                              
      Loop
         Begin            
            Error := 'false'
            Cmd := 'fetch book'
            Step := '001'
            Id_count  : = 0;
            Tsn         := ' ';
            Last_ids     :='  ' ;  
   
  if score = A then
     fetch book 1 into ssn,year,name
         if book1%notfound then 
  exit;
endif;
   elsif score = B  then
       fetch book 2 into ssn, year,name
           if book2%not found then
exit;
endif;
   elsif core =c then
        fetch book3 into ssn,year,name;
            if book3%not found then
exit;
endif;[/b]

[Updated on: Wed, 16 December 2009 10:38]

Report message to a moderator

Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435542 is a reply to message #435534] Wed, 16 December 2009 11:17 Go to previous messageGo to next message
cookiemonster
Messages: 12403
Registered: September 2008
Location: Rainy Manchester
Senior Member
If you want us to check your code it helps to post code that compiles. What you've posted is absolutely riddled with compile errors.
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435543 is a reply to message #435534] Wed, 16 December 2009 11:30 Go to previous messageGo to next message
BlackSwan
Messages: 25033
Registered: January 2009
Location: SoCal
Senior Member
> // I think the last 2 are repeating or can be combined
Maybe the can be combined or maybe not.
Since we don't have your data & since you have not specified any requirements, we can't draw ANY conclusion.

It would be helpful if you provided DDL for tables involved.
It would be helpful if you provided DML for test data.
It would be helpful if you provided expected/desired results & a detailed explanation how & why the test data gets transformed or organized.

Post Operating System (OS) name & version for DB server system.
Post results of
SELECT * from v$version
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435550 is a reply to message #435542] Wed, 16 December 2009 12:30 Go to previous messageGo to next message
sjackson1972
Messages: 13
Registered: December 2009
Junior Member
Sorry, I was unaware that what I had put down could not be compiled. This is actually part of a long ongoing procedure that was printed out to me to evaluate, so I only have a soft copy. I was only told to see if there were any statements that I could possible combine into 1....

No other data..ie ddl or data has been given to me.
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435558 is a reply to message #435534] Wed, 16 December 2009 16:19 Go to previous messageGo to next message
cookiemonster
Messages: 12403
Registered: September 2008
Location: Rainy Manchester
Senior Member
Somebody wants you to work this out on paper? Shocked
Talk about making life hard.
And if you really want to simplify code, knowing the table structures of the tables you're working with is a basic essential.

It looks there's some repetition that could maybe be done away with but the code snippet is too short to tell for sure.
It could be you could really simplify it by combining the three cursors into one but since you haven't posted the cursor definitions we can't tell.
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435560 is a reply to message #435558] Wed, 16 December 2009 16:51 Go to previous messageGo to next message
sjackson1972
Messages: 13
Registered: December 2009
Junior Member
i have already combined the cursors after i posted the question.
the only thing i have so far is a print out of the field names data types and table names. I don't have an er diagram or anything, so basically i'm just shooting from the hip hoping I can get help..This procedure is about 9 pages long and I'm trying to analyze the code bit by bit. I was thinking that I could change some of the if/elsif statements to case statements..
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435561 is a reply to message #435558] Wed, 16 December 2009 16:52 Go to previous messageGo to next message
ThomasG
Messages: 3189
Registered: April 2005
Location: Heilbronn, Germany
Senior Member
Or, to put it in another way:

If there was a way to optimize code based only on the code itself, without taking into consideration the requirements and the data model, then most of the people in this forum would already have been replaced by some cleverly written shell scripts. Very Happy

The "I have $Problem, is there a better solution that this code here?" is most of the time more worth asking that "I have this code here, can I make it better?"
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435562 is a reply to message #435561] Wed, 16 December 2009 16:57 Go to previous messageGo to next message
ThomasG
Messages: 3189
Registered: April 2005
Location: Heilbronn, Germany
Senior Member
I could be completely off on this one, but it MIGHT be possible to actually change the cursor so that the if/elsif statements are implemented in the where clause of the cursor itself, so you could get rid of the entire portion of code you just posted.

Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435563 is a reply to message #435560] Wed, 16 December 2009 17:14 Go to previous messageGo to next message
BlackSwan
Messages: 25033
Registered: January 2009
Location: SoCal
Senior Member
>I don't have an er diagram or anything, so basically i'm just shooting from the hip hoping I can get help.
Ready, Fire, AIM!

>This procedure is about 9 pages long and I'm trying to analyze the code bit by bit. I was thinking that I could change some of the if/elsif statements to case statements..

If you don't know where actual time is being spent you could be trying to optimize already efficient code.

Another perspective is that PL/SQL code runs at CPU/processor speed while SQL runs at disk speed or 1000 TIMES slower.

Perhaps the procedure is slow due to poor SQL & not "redundant" PL/SQL code.

Of course without actual facts such as those produced by
ALTER SESSION SET SQL_TRACE=TRUE
you are really just shooting in the dark while hoping to get lucky

Good Luck!
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435564 is a reply to message #435534] Wed, 16 December 2009 18:09 Go to previous messageGo to next message
sjackson1972
Messages: 13
Registered: December 2009
Junior Member
I don't think its the time it takes for the code to run.
Thanks for your help though..
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435567 is a reply to message #435564] Wed, 16 December 2009 18:46 Go to previous messageGo to next message
BlackSwan
Messages: 25033
Registered: January 2009
Location: SoCal
Senior Member
>I don't think its the time it takes for the code to run.
Now I am curious.
If you are not trying to make the code run faster, then what is the purpose of your efforts?
Based upon what criteria would an independent observer conclude this task was successfully completed?
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435568 is a reply to message #435567] Wed, 16 December 2009 19:36 Go to previous messageGo to next message
sjackson1972
Messages: 13
Registered: December 2009
Junior Member
The code was written back in 1994 (oracle 8i). The client feels that the code written is to long. I posted in a previous post that the manager handed me 54 pages worth of pl/sql code. Its a pl/sql package with about 9 or more procedures doing this execution. They just basically just want a shorter(condensed)version of what they have been using in the past. My job is to see if some of the sql statements,procedure,loops etc can be consolidated. So, i'm guessing instead of having 10 procedures, 30 loops etc..they want me to clean up the code that is already there . So, i'm going through all 54 pages bit by bit or should i say procedure by procedure and trying to make each procedure shorter but at the sametime obtaining the same results/output.(this I will not know until my system gets set up)
Now, my resources are limited as you can tell, so I had to seek help from this forum. So, in a nutshell i guess i'm taking what is already done and putting the long version into a short version. I do know that they will be running this in Oracle 11, so anything as far as syntax errors or code not compiling will come later.

for example here is an example of 2 of the queries written in a cursor

Select ta.country,ta.id, sa.city, sa.state,s.zipcode
from site ta, location sa, area s
where process in 9
and s.site_seq = ta.site_seq
and sa.site_nbr=ta.site_nbr
and ta.datefile is not null
and ta.movefile is not null
and to_char(ta.datetime,'yyyy')=this_year
order by id,ta. country,s.zipcode,sa.city


Select ta.country,ta.id, sa.city, sa.state,s.zipcode
from site ta, location sa, area s
where process in 9
and s.site_seq = ta.site_seq
and sa.site_nbr=ta.site_nbr
and (ta.datefile is null or ta.autostep is null)
and to_char(ta.datetime,'yyyy')=this_year
order by id,ta. country,s.zipcode,sa.city

I hope this makes since.

[Updated on: Wed, 16 December 2009 19:39]

Report message to a moderator

Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435579 is a reply to message #435568] Wed, 16 December 2009 23:19 Go to previous messageGo to next message
Frank
Messages: 7880
Registered: March 2000
Senior Member
If this is 8i code, they are plain lucky the code never failed.
All the unconditional dbms_output calls might very well have caused a buffer overflow in the dbms_output buffer.
It is a bad habit to have such (unconditional) calls in production code, so I would advise to create some logging procedure that makes it possible to switch logging on or off from the outside.

Furthermore, how about rewriting this 9(!) page long procedure into something more structured, like a package? That will definitely improve the readability of it plus it will ease the task of reusing pieces of code. To not break existing code, the original procedure can be a wrapper for the package.
But said all this, I agree with the comments of others: having to alter this with only a hard copy is insane.
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435593 is a reply to message #435568] Thu, 17 December 2009 00:32 Go to previous messageGo to next message
Littlefoot
Messages: 20888
Registered: June 2005
Location: Croatia, Europe
Senior Member
Account Moderator
sjackson1972 wrote on Thu, 17 December 2009 02:36
The manager handed me 54 pages worth of pl/sql code. They just basically just want a shorter (condensed) version of what they have

How about printing the same code with a smaller font (let's say Courier size 7)? It might turn 54 pages into 30 or so.
./fa/1599/0/
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435595 is a reply to message #435593] Thu, 17 December 2009 00:35 Go to previous messageGo to next message
BlackSwan
Messages: 25033
Registered: January 2009
Location: SoCal
Senior Member
Littlefoot wrote on Wed, 16 December 2009 22:32
sjackson1972 wrote on Thu, 17 December 2009 02:36
The manager handed me 54 pages worth of pl/sql code. They just basically just want a shorter (condensed) version of what they have

How about printing the same code with a smaller font (let's say Courier size 7)? It might turn 54 pages into 30 or so.
./fa/1599/0/

If you did 4 pages per side it would be down to 4 or 5 sheets with a double sided printer.
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435611 is a reply to message #435593] Thu, 17 December 2009 02:10 Go to previous messageGo to next message
Frank
Messages: 7880
Registered: March 2000
Senior Member
Littlefoot wrote on Thu, 17 December 2009 07:32
sjackson1972 wrote on Thu, 17 December 2009 02:36
The manager handed me 54 pages worth of pl/sql code. They just basically just want a shorter (condensed) version of what they have

How about printing the same code with a smaller font (let's say Courier size 7)? It might turn 54 pages into 30 or so.
./fa/1599/0/

Or how about handing the same printout back and tell him you made your alterations. If he wants the diffs, he can compare the two hardcopies
Re: Is this statement repeating itself (code review) or can the last 2 be combined [message #435636 is a reply to message #435611] Thu, 17 December 2009 03:04 Go to previous message
ThomasG
Messages: 3189
Registered: April 2005
Location: Heilbronn, Germany
Senior Member
Or, if it's only a print-out, then delete every second line, and when it doesn't compile blame it on the persons that typed the print-out into the database again. Laughing

Without at least SOME specification about what the package actually does, AND a electronic copy of the script you should charge a hefty extra fee for reverse-engineering what it actually does.
Previous Topic: Check if current procedure/function is running
Next Topic: Time Bucket
Goto Forum:
  


Current Time: Sat Dec 03 12:28:39 CST 2016

Total time taken to generate the page: 0.17126 seconds