Code refactoring…



By Patch ~ December 12th, 2008. Filed under: Oracle, Tools.

Steven Feuerstein wrote an entry on his Toadworld blog where he checks the refactoring capabilities of SQL-Developer.

I am trying the same things in PL/SQL Developer to see if this does do the trick.

First of all, a different couple of options are presented to you when you wrong click (or right click) a piece of highlighted code, depending on what you selected.

refactoring_menu

Let’s take the following code as our starting point:

create or replace procedure use_cursor (
   security_level_in in     pls_integer
,  cursor_out        in out number
)
authid definer
is
begin
   cursor_out := dbms_sql.open_cursor(security_level_in);
   
   dbms_sql.parse ( cursor_out
                  , 'select count(*) from all_source'
                  , dbms_sql.native
                  );
end;

Show Image

Now select the first line of the executable section of our code and choose ‘Extract procedure…’ from the popup menu:refactoring_menu_extract_procedure

We are now presented with the following dialog to name the new procedure:

New_procedure_name

After pressing the OK button, our code is changed to:

create or replace procedure use_cursor (
   security_level_in in     pls_integer
,  cursor_out        in out number
)
authid definer
is
  -- Refactored procedure open_cursor_with_sl 
  procedure open_cursor_with_sl(security_level_in in pls_integer, cursor_out in out number) is
  begin
    cursor_out := dbms_sql.open_cursor(security_level_in);
  end open_cursor_with_sl;
begin
   open_cursor_with_sl(security_level_in, cursor_out);
   
   dbms_sql.parse ( cursor_out
                  , 'select count(*) from all_source'
                  , dbms_sql.native
                  );
end;

Show Image

It seems to do it’s work like it’s supposed to. It even compiles, which makes it tested ;-)

Now, let’s refactor some variable names. PL/SQL Developer wants you to highlight the first occurrence of the variable, for it to enable the ‘Rename item…’ option.

Using this code as a starting point:

create or replace procedure use_cursor (
   security_level_in in     pls_integer
)
authid definer
is
   l_cursor number;
begin
   l_cursor := dbms_sql.open_cursor(security_level_in);
   
   dbms_sql.parse ( l_cursor
                  , 'select count(*) from all_source'
                  , dbms_sql.native
                  );
                 
   dbms_sql.close_cursor(l_cursor);
end;

Show image

after selecting the first occurrence of l_cursor and selecting ‘Rename item…’ from the popup menu we are presented with the following dialog:

Rename

After typing in the new name of the variable, the code looks like this:

create or replace procedure use_cursor (
   security_level_in in     pls_integer
)
authid definer
is
   l_dyn_cursor number;
begin
   l_dyn_cursor := dbms_sql.open_cursor(security_level_in);
   
   dbms_sql.parse ( l_dyn_cursor
                  , 'select count(*) from all_source'
                  , dbms_sql.native
                  );
                 
   dbms_sql.close_cursor(l_dyn_cursor);
end;

Show image

Now, let’s add a reference to a package variable with the same name as we just changed the variable to:

create or replace procedure use_cursor (
   security_level_in in     pls_integer
)
authid definer
is
   l_dyn_cursor number;
begin
   l_dyn_cursor := dbms_sql.open_cursor(security_level_in);
   
   dbms_sql.parse ( l_dyn_cursor
                  , 'select count(*) from all_source'
                  , dbms_sql.native
                  );
                 
   dbms_sql.close_cursor(l_dyn_cursor);
   
   my_package.l_dyn_cursor := 10;
end;

Show image

When the ‘Rename item…’ option is selected, the same dialog is displayed as before (makes sense, we are doing pretty much the same thing):

Rename2

After the refactoring the code looks like this:

create or replace procedure use_cursor (
   security_level_in in     pls_integer
)
authid definer
is
   l_dyn_cursor2 number;
begin
   l_dyn_cursor2 := dbms_sql.open_cursor(security_level_in);
   
   dbms_sql.parse ( l_dyn_cursor2
                  , 'select count(*) from all_source'
                  , dbms_sql.native
                  );
                 
   dbms_sql.close_cursor(l_dyn_cursor2);
   
   my_package.l_dyn_cursor := 10;
end;

Show image

As you can see, it didn’t touch the variable from the package, even though it has the same name as the variable being refactored. There is no option to change the case of the selection, but I have to agree with Steven on this one, that’s no real refactoring, more a layout issue.

Reader's Comments

  1. Steven Feuerstein | December 12th, 2008 at 9:33 pm

    Thanks, Patrick! Looks like you did my work for me for my next blog entry on PL/SQL Developer. I will just copy and paste what you have done. :-)

  2. Vadim Tropashko | December 15th, 2008 at 10:54 pm

    Regarding Steven’s article on SQLDeveloper,
    apparently he did all the testing from sql worksheet, which (alas!) slipped past out qa. SQL worksheet doesn’t have full PL/SQL functionality, this is why most of the people use PL/SQL editor, which doesn’t have most of these problems.

    Now, have you tried to select a syntactically invalid fragment in PL?SQL developer? For example,
    the first three lines in

    SELECT 100 INTO x FROM dual d;
    IF i=x THEN
    NULL;
    END IF;

    PL/SQL developer would be able to refactor it!

  3. Steven Feuerstein | December 19th, 2008 at 7:52 pm

    Vadim, thanks for pointing out an issue with how I reviewed SQL Dev. In the future, please do post a comment over at my blog as well!

    As for your comment about refactoring of an invalid fragment…yes, it is true that PL/SQL Dev does refactor it, but it does not pick up the END IF; that was not highlighted. Maybe that is what you were expecting; I was hoping it would pull the rest of the statement needed to make it valid….

    SF

  4. Vadim Tropashko | December 19th, 2008 at 10:27 pm

    Steven, yes, it is awkward to have a conversation here instead of your blog. However, one can’t post comments there, until registered, and this is a showstopper for me.

Leave a Comment