Code Comment WTF? Part 209 Found this in a snippet today:

-- ********************************
-- End of Package Body

END package_pkg ;
/

Seriously? Was that necessary? Could I possibly be under the illusion that it is not the end of the package?

Stop it.

Now.

Labels: code, funny, style, wtf

  ¶ 3/09/2010 01:38:00 AM   6 Comments Links to this post

  How To Kill a Code Review Today's guest post is from Gary Myers from Igor's Oracle Lab. I was first introduced to Gary via comments left here. I can't find the first one of course...but he has left plenty of them. All well thought out and informative. Most recently he introduced me to the ability to do a bulk bind using %ROWTYPE here.

This is a topic that is all too often ignored, as you all know.

Steven Feuerstein states here that "Everyone knows that code review is a good idea"

The problem is what happens after the review.

Once upon a time there was a piece of code. That code had been in production for a long time, ran pretty slow but not slow enough that it had reached the top of the pile of stuff people complained about.

A change was done to that code for a new enhancement, and in one of those bursts of enthusiasm that sometimes hits a development team, it got subjected to a code review.

The whole structure of that code was ugly, with unnecessary nested cursor loops. The big kicker for performance was that at the end of one of these inner loops was a TRUNCATE TABLE. Because when you are deleting all the rows from a table, every-one *KNOWS* that a truncate is fastest, right ? Of course, as the TRUNCATE is DDL, it meant that all the SQL using that table inside that loop was getting re-parsed each and every time through the loop.

There were other problems in the code too. I believe one was with variables not being re-initialized at various points in the loop, so there was a risk of incorrect results in some unlikely cases.

The verdict of the review was that the code needed a re-write. The problem was, since it was already in production, no-one wanted to admit that there were bugs in it (and the users hadn't spotted any incorrect results). The new code would go into a future patch, but that wouldn't go live for months. However it had been promised for delivery to a test environment. A rewrite would mean missing the drop deadline.

A quick-fix could be done to improve performance. A rewrite could be done and the deadline missed. The quick-fix could be done and still meet the deadline, then a later rewrite to fix the underlying problems (but those problems probably would have been blamed on the quick fix).

A compromise was reached. Since the change to the code didn't actually introduce any new bugs, it would be allowed to go through to test with no changes from the review. And there was a promise to actually rewrite the code.

Of course, once the delivery was done, lots of other priorities came ahead. I don't know whether the code ever got the rewrite, but I suspect not. Definitely, for at least six months, there was a batch job taking hours when a five minute code change could have cut it to minutes.

At least the developers who participated in the review learnt that a TRUNCATE has drawbacks. The code reviews pretty much never happened again though.

Labels: code, development, discipline, gmyers

  ¶ 8/23/2009 09:52:00 PM   7 Comments Links to this post

  Writing Maintainable Code The last few years I've inherited quite a bit of code. The vast majority of it reads terribly.

I get newbies; those who have yet to find their style. Those that I run into I highly encourage to find a style and stick with it...even if the commas are in the front of the line. Of course I wish my style would be the standard. Wait a minute, it already is. Check out this snippet from the Official Oracle documentation:



Let me live in my own fantasy world...

What about the rest though? A friend recently told me he thinks it takes about 2 years for someone to develop their own distinctive style. So why doesn't it seem to happen? Or, do I just see all newbie code?

I doubt that's the case.

I try to write code so that it is readable. Others may not like my style, but it is consistent. A former colleague once said that my code (a 5,000 line ETL package) read very easily. It had a natural flow to it. That was one of the best compliments I have ever received. The point is, I write code with the assumption that someone else will (have to) read it.

I find myself in SQL*Plus, ad-hoc queries, writing them the way I would if I were to check it into source control.

SELECT OWNER, TABLE_NAME FROM DBA_TABLES

...that's pretty rare. I find myself backspacing to fix it

SELECT owner, table_name
FROM dba_tables

That might not be the best example (I have a script for that anyway), but you get the point.

"Practice like you play" is a common sports phrase. Perhaps that's why I write ad-hoc queries in that fashion.

I would offer tips, but that would just lead me back to style. I don't want to force people into using my style (that's a lie, but I accept that I am not Supreme Ruler of Code Style), I just want them to choose.

At a minimum, run your code through one of the various code formatters. Toad has one, SQL Developer has one, I think most tools have one. You can even set them up to match your personal preferences (which should align perfectly with mine).

When asking for help from someone else, don't use the default font in your email client (unless your a plain text email person), highlight the code and put it in Courier New. Please.

Finally, write your code as if someone else has to read it!

Labels: code, style

  ¶ 8/21/2009 12:01:00 AM   5 Comments Links to this post

  Alerts - Scheduler Jobs Did Not Run The last couple of days our warehouse environment hasn't been updating properly. It's basically a collection of Materialized Views based on the OLTP system. It appears as though many (all?) of our scheduler jobs are not running. DBA believes it has something to do with the Flash Recovery Area running out of space. I don't know and I don't have the time to find a root cause. I do know that there were no errors and the jobs were not in a failed status.

Rather than have the business inform us their reports our not right I was tasked with creating an alert if the jobs did not run. Usually we set up alerts for things that break or actually completed. I think this is the first time I've had to build something to the opposite.

Here's what I came up with:

CREATE OR REPLACE
PROCEDURE jobs_did_not_run
AS
  b VARCHAR2(3) := CHR(10);
  TYPE r_table IS RECORD
  (
    job_name VARCHAR2(30),
    next_run_date DATE,
    job_action VARCHAR2(4000)
  );
  TYPE t_table IS TABLE OF R_TABLE;
  l_table T_TABLE := T_TABLE();
  l_subject VARCHAR2(50) := 'Alert - Jobs have not run';
  l_message VARCHAR2(32767);
BEGIN
  SELECT job_name, next_run_date, job_action
  BULK COLLECT INTO l_table
  FROM dba_scheduler_jobs
  WHERE state = 'SCHEDULED'
    AND enabled = 'TRUE'
    AND next_run_date < SYSDATE;
 
  l_message := 'The following jobs have not run today:' || b;
  FOR i IN 1..l_table.COUNT LOOP
    l_message := l_message || 'Job:           ' || 
                   l_table(i).job_name || b;
    l_message := l_message || 'Next Run Date: ' || 
                   TO_CHAR( l_table(i).next_run_date, 'MM/DD/YYYY HH24:MI:SS' ) || b;
    l_message := l_message || 'Action:        ' || 
                   l_table(i).job_action || b;
  END LOOP;

  utl_mail.send
    ( sender => '',
      recipients => '',
      subject => l_subject,
      message => l_message );

END jobs_did_not_run;
/
show errors

You can find it here in my Google Code home for DBA Utilities. I do believe that at some future point this will be incorporated into a package, but for now it is a standalone procedure.

You'll receive a nice little email noting the JOB_NAME, scheduled NEXT_RUN_DATE and the JOB_ACTION (the anonymous block).

Labels: code, dba, open source

  ¶ 3/06/2009 11:57:00 PM   0 Comments Links to this post

  Google Code I've create a project on Google Code where I plan to start putting everything that I create, whether I blog about it or not.

http://code.google.com/p/oraclenerd/

I'm trying to figure out if I should do it project based or schema based...or application based? Don't really know.

I'll probably start with the Poor Man's Data Vault.

Google has some pretty cool stuff in their apps department there:
http://code.google.com/more/#products-products-android

I'd definitely like to check out the Chart API


http://chart.apis.google.com/chart?cht=p3&chd=t:60,40&chs=250x100&chl=Hello|World

How cool is that?

Also want to check out their Visualization API. A nice example can be found here.

So I strayed?

Has anyone else out there used Google Code?

Labels: code, development, google

  ¶ 2/26/2009 10:36:00 PM   1 Comments Links to this post

  %TYPE, What's the Point? As I started to read more and more I found many people were advocated the use of %TYPE in variable/parameter declarations. I thought, "Great, I should do that too!"

So after a few years of using them I have something of an opinion on them.

Pros

1. Strongly typed declarations


2. Inheritance - If the column data type changes, you don't have to change any of your packaged code (not really sure if that is different than #1)

Cons

1. Difficult to debug - What data type was APPLICATION_DETAIL.FOOID or worse, what was SCHEMANAME.APPLICATION_DETAIL.FOOID? Is it a NUMBER, VARCHAR2, or something else? Off to SQL Developer or SQL*Plus to do a describe on the table...I once spent a full day trying to figure out which of the 30 passed in parameters (and their values) was throwing a data type error. Another developer finally found it.


2. Too much typing - I love to type. Seriously. I'm going to say it...this is too much typing.


3. It's Ugly - Alright, that's not really a con is it? I like my code pretty. Many times using the SCHEMANAME.APPLICATION_DETAIL.FOOID%TYPE takes me over 90 characters wide...it's just ugly.

Wow, I guess that's not much of a list is it?

In a development situation or trying to spec something out, I can definitely see the value. Nothing is set in stone and needs to be somewhat fluid. But in a stable production environment? Is it really necessary? If you are going to change the data type or precision of a column, wouldn't you expect to make a few changes? For some reason I think of the need to change your Primary Key, and all references...

%TYPE has it's merits. But I think the love affair is over for me.

Labels: code, plsql

  ¶ 10/08/2008 10:13:00 PM   8 Comments Links to this post

  How Does Oracle Make Development Easier? Continuing on the theme of late, what are the basic things that you can do to reduce the amount of code that needs to be written?

In the post linked above, I mentioned Constraints as probably the easiest way to reduce the amount of coding. For example:

CREATE TABLE t
(
  id NUMBER(10)
    CONSTRAINT pk_id_t PRIMARY KEY,
  first_name VARCHAR2(30)
    CONSTRAINT nn_firstname_t NOT NULL,
  middle_name VARCHAR2(30),
  last_name VARCHAR2(40)
    CONSTRAINT nn_lastname_t NOT NULL,
  gender VARCHAR2(1)
    CONSTRAINT nn_gender_t NOT NULL
    CONSTRAINT ck_morf_gender_t CHECK ( gender IN ( 'M', 'F' ) ),
  ssn VARCHAR2(9)
    CONSTRAINT nn_ssn_t NOT NULL
    CONSTRAINT ck_9_ssn_t CHECK ( LENGTH( ssn ) = 9 )
    CONSTRAINT ck_numeric_ssn_t CHECK ( REGEXP_INSTR( ssn, ?, ?, ? ) )
    CONSTRAINT uq_ssn_t UNIQUE  
);

ID - is just a sequence generated key, no big deal there.
FIRST_NAME - is not optional you hence the NOT NULL constraint.
MIDDLE_NAME - is optional (no constraint).
LAST_NAME - is not optional (NOT NULL).
GENDER - is not optional (NOT NULL). Also, you want to exclude everything but 'M' or 'F', thus the CHECK constraint.
SSN - is not optional (NOT NULL). The length of the value must be 9 characters (CHECK). The characters may only be numeric (CHECK). Unfortunately I don't yet know the REGEXP_INSTR function yet to truly demonstrate. Finally, the UNIQUE constraint on SSN since they shouldn't duplicate across people.

This is a simple demonstration of how you can potentially use constraints to reduce the amount of code necessary. Though I would probably check/validate these as well in code because the error that is generated will not be unique so it would difficult to tell. The point is, if you make a mistake in your validation code it will be easily caught by the constraints forcing you to fix it.

This will give you much more reliable data, which as we all know, is the most important thing.

I'd like to do more of the posts pointing out the easiest methods to reduce the amount of code you have to write by using Oracle.

What kind of solutions do you have or do you use?

Labels: code, constraints, database, oracle, simplicity

  ¶ 3/27/2008 10:28:00 PM   0 Comments Links to this post

  Instrumentation: DEBUG/LOGGING In my previous entry on instrumenting code I detailed the way in which you could use the DBMS_APPLICATION_INFO.SET_SESSION_LONGOPS procedure to help monitor your long running code.

In this one, I will detail my home grown version of Tom Kyte's debug routine. I do know that others have similar code but can't seem to find them right now.

You can find the source code here.

Contents:
2 tables
2 sequences
1 package
1 procedure
1 build file

debug_tab allows you to turn the debugging on and off. debug_details_tab will store each line that you write to the debug routine when turned on.

Here's an example of it in practice:

CREATE OR REPLACE
FUNCTION get_age_in_months( p_id IN NUMBER ) RETURN NUMBER
IS
  l_age_in_months INTEGER;
BEGIN
  --instrumentation calls
  debug( 'GET_AGE_IN_MONTHS' );
  debug( 'P_ID:  ' || p_id );
  debug( 'select value into variable' );

  SELECT age_in_months
  INTO l_age_in_months
  FROM people_tab
  WHERE id = p_id;

  debug( 'L_AGE_IN_MONTHS:  ' || l_age_in_months );

  RETURN l_age_in_months;

EXCEPTION
  WHEN no_data_found THEN
    debug( 'no data found' );
    RETURN NULL;
END get_age_in_months;
/
show errors

I mentioned in the previous article that I had had difficulty grasping this concept initially. I think once I related it to DBMS_OUTPUT.PUT_LINE it became much more clear to me.

This simple debug routine has helped me tremendously in the last year or two that I have used it. Especially when you get nested levels of logic. It gets very hard to keep track of where you are, but with this you can run your procedure or function and then issue a SELECT against the debug_details_tab and see what was happening when.

I even began using this in my APEX applications. I would preface each line with "APEX: " and then write whatever was necessary so that I could step through the various pieces of code. It became crucial when I was doing validation on a table of records in a collection...oh so much easier.

On large systems this will generate a lot of data. I definitely would not put this inside a loop doing some sort of batch processing, but it is helpful to find where things fail out.

It can certainly be improved on. I basically took the idea of DBMS_OUTPUT.PUT_LINE and wrote it to a table, nothing fancy there. Mr. Kyte mentions writing it to the file system as well. Since I don't have ready access to the database file system, this was the easiest.

Make your life easier and use a debug/logging routine in your code. No longer will you have to comment, the debug statements should do it for you!

Labels: code, development, howto, instrumentation, plsql, tools, utilities, work

  ¶ 12/05/2007 09:03:00 PM   1 Comments Links to this post

  Code Style: Functions Functions

Functions are used to return a value. This value can take the form of a set of records (ref cursor), a collection, or a single value (NUMBER, VARCHAR2, etc.).

Functions (or procedures for that matter) should be logically grouped within a package. Only on the rare occasion should you store them individually.

That said, here is the syntax to create a function in Oracle:

CREATE OR REPLACE
FUNCTION get_age_in_months( p_id IN NUMBER ) RETURN NUMBER
IS
  l_age_in_months INTEGER;
BEGIN
  --instrumentation calls
  debug( 'GET_AGE_IN_MONTHS' );
  debug( 'P_ID:  ' || p_id );
  debug( 'select value into variable' );

  SELECT age_in_months
  INTO l_age_in_months
  FROM people_tab
  WHERE id = p_id;

  debug( 'L_AGE_IN_MONTHS:  ' || l_age_in_months );

  RETURN l_age_in_months;

EXCEPTION
  WHEN no_data_found THEN
    debug( 'no data found' );
    --here you need to decide what exactly you want to do.  If this is 
    --used in a SQL statement then you probably don't want to halt 
    --processing, so returning NULL will work just fine. However, if 
    --this function is called by another function or procedure and you 
    --need this value to continue processing, you WOULD want to know
    --that the value is not there and you would issue a RAISE after you've
    --logged the error
    RETURN NULL;
END get_age_in_months;
/
show errors

The "debug" call is just instrumentation of the code. It will allow you to step through the code more easily.

In the above function, you would also need to be aware of too_many_rows, but since I know that "id" is the primary key on the table, I have ommitted it.

1. Instrument your code.


2. Name the function something descriptive. There is a 30 character limit so don't be lazy.


3. Name your functions something descriptive. I typically use get_ then whatever it is I'm doing. get_calculated_amount, get_as_of_date, etc.


4. Name your variables something descriptive. There is a 30 character limit so don't be lazy. Please just spell out "no". Is it "No" or "Number?" Why make the next person think, just spell it out. They'll thank you for it.


5. If there are more than 1 input parameters, start at the next line.


6. CREATE OR REPLACE goes on the top line, nothing else


7. Use spaces liberally


8. Comment where necessary. If you name things descriptively though, you'll find you won't need a lot of comments.

I tend to name variables thusly (I think I got that from Mr. Kyte):

• p_ = parameters passed (though you could use i_ for in and o_ for out as well)


• l_ = local variables


• g_ = global variables

Labels: code, development, style

  ¶ 10/31/2007 10:59:00 PM   2 Comments Links to this post

  Code Style: Package Specification Package Specification

CREATE OR REPLACE
PACKAGE p_foo
AS
  FUNCTION get_something
    ( p_id IN NUMBER,
      p_thing_im_looking_for VARCHAR2 ) RETURN NUMBER;
  PROCEDURE do_something( p_id IN NUMBER );
  PROCEDURE insert_something
    ( p_first_name IN PEOPLE_TAB.FIRST_NAME%TYPE,
      p_last_name IN PEOPLE_TAB.LAST_NAME%TYPE,
      p_email_address IN PEOPLE_TAB.EMAIL_ADDRESS%TYPE,
      p_middle_initial IN PEOPLE_TAB.MIDDLE_INITIAL%TYPE DEFAULT NULL,
      p_prefix IN PEOPLE_TAB.PREFIX%TYPE DEFAULT NULL );
  PROCEDURE update_something
    ( p_id IN NUMBER,
      p_first_name IN VARCHAR2,
      p_last_name IN VARCHAR2 );
  PROCEDURE delete_something( p_id IN NUMBER );
END p_foo;
/
show errors

1. Name your package something descriptive. There is a 30 character limit so don't be lazy.


2. Name your procedures and functions something descriptive. There is a 30 character limit so don't be lazy. UPDATE this or INSERT that. Try to give a small clue as to what it does. If it's a function, I typically use get_ then whatever it is I'm doing. get_calculated_amount, get_as_of_date, etc.


3. Name your variables something descriptive. There is a 30 character limit so don't be lazy. Please just spell out "no". Is it "No" or "Number?" Why make the next person think, just spell it out. They'll thank you for it.


4. Use the package name after the END statement.


5. If there are more than 1 input parameter, start at the next line.


6. CREATE OR REPLACE goes on the top line, nothing else


7. Use spaces liberally


8. Comment where necessary. If you name things descriptively though, you'll find you won't need a lot of comments.


9. Declare parameters using %TYPE. If the parameter doesn't map to a table column declare SUBTYPEs somewhere.

Labels: code, development, style

  ¶ 10/11/2007 10:16:00 PM   2 Comments Links to this post

  Code Style: Index My ongoing contribution to...pretty much myself. Perhaps you can find this useful, or amusing, whichever.

• General Principles


• Tables


• Functions


• Package Specification

Labels: code, development, style

  ¶ 10/11/2007 01:00:00 AM   0 Comments Links to this post

  Code Style: Tables Tables are easy.

CREATE TABLE t
(
  col1 NUMBER(10,0)
    CONSTRAINT  pk_col1 PRIMARY KEY,
  col2 VARCHAR2(32)
    CONSTRAINT nn_col2_t NOT NULL
    CONSTRAINT uq_col2_t UNIQUE,
  col3 VARCHAR2(400),
  col4 VARCHAR2(1) DEFAULT 'N'
    CONSTRAINT ck_yorn_col4_t CHECK ( col4 IN ( 'Y', 'N' ) )
    CONSTRAINT nn_col4_t NOT NULL
);

Remember to always name your constraints. While I am at, use constraints as much as humanly possible, at least in your OLTP systems. You'll be able to reduce the amount of code you need to write and actually let the database do it's job. I'd much rather let the database do it than rely on code to maintain my data integrity.

For the datawarehouse, you'll need to think about constraints a bit more as it may slow down load times. I'm still all for constraints, but I would never say always use them.

For child tables:

CREATE TABLE s
(
  col5 NUMBER(10,0)
    CONSTRAINT pk_col5 PRIMARY KEY,
  col1
    CONSTRAINT fk_col1_s REFERENCES t( col1 )
    CONSTRAINT nn_col1_s NOT NULL,
  col6 VARCHAR2(30)
);

For Foreign Key constraints, you do not have to declare the type as it will be inherited from the parent table.

This would be helpful if someone up and decided to change the NUMBER(10,0) to a VARCHAR2(10) or something (please don't ever do that!).

As for STORAGE or other table options, I typically leave that up to the DBA or work with them to add them. They may have a particular setup for certain tables that you can't possibly know (if you don't talk to them).

To recap:

• Use constraints as much as possible


• Always name your constraints


• Work with your DBA for table options


• Always name your constraints

Labels: code, constraints, development, discipline, style

  ¶ 10/11/2007 12:37:00 AM   0 Comments Links to this post