Metacode Gone Wrong

Following on from my reply about code comments at AskTom, here is a lovely example from The Daily WTF of how not to do it. It looks like we have something to learn from both ends of the spectrum.

Does nobody do code reviews any more?

About these ads

7 thoughts on “Metacode Gone Wrong

  1. /*
    || :META: selects employee
    || :META: inserts employee
    || :META: updates department
    || :META: updates address
    || :META: calls is_number
    || :META: calls log_entry
    || :META: calls dbms_application_info
    || :META: changes global_counter
    */

    With a little bit of string parsing, this stuff *could* be automated. I’m of the opinion that what can’t be answered with automation is WHY. You can discover all the servers on my network, and get CPU and memory and what processes it runs, but you can’t tell why it runs them or how important it is to my business. In your example, somewhere in that proc, global_counter either exists on the left of := or after an INTO. Code can parse that out. WHY did you increment global_counter is something the next person may not understand.

    But yeh, no one does code reviews anymore.

    • Yes, if you know what you’re looking for I think that most of those are pretty straightforward. But there are lots of less straightforward cases to consider, such as multitable insert, line-breaks in odd places … etc..

      One issue that has always bothered me a little with PL/SQL is breaking out the code into procedures and functions and getting an overview of the code. I think that tagging the beginning of procedures and functions and the important functionality in them would let you get a reasonable overview with a query against user_source …

      || :META: Package pile_of_stuff
      || :META: section private
      || :META: Procedure change_employee_address
      || :META: selects employee
      || :META: inserts employee
      || :META: updates department
      || :META: updates address
      || :META: section public
      || :META: Function log_operation
      || :META: calls is_number
      || :META: calls log_entry
      || :META: calls dbms_application_info
      || :META: changes global_counter

      I’m eating my own dog food on this at the moment, so I’ll see how it works out. Maybe I’ll be able to post a “metacode dump” and we’ll see if it makes sense.

      • Don’t get me wrong, I’m not arguing against your format. I love it. But in a Code Comment Hierarchy of Needs, why is the most important, what is second. I don’t know — Third Base!

  2. But there are lots of less straightforward cases to consider, such as multitable insert, line-breaks in odd places … etc…

    What about dumping the source (USER/ALL/DBA_SOURCE) into a clob for each object? You would then be able to avoid the line-break issue if using regular expressions. I got that piece done here, but I did not get around to learning the width and breadth of regular expressions (as intended).

    My goal was similar to yours in that I was hoping to have a more fine-grained look at what the package was doing. I know from %_DEPENDENCIES that table MY_TABLE is referenced, but in what way? INSERT, UPDATE, SELECT or DELETE?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s