RTFM? KISS? Comment? Whatever, just get the code to work right!

We've been hitting a snag of late with new registrants at the PL/SQL Challenge. As with many sites, to ensure that a person's email is not being hijacked, we send an email with a verification URL.

It was working for quite a while, but then we noticed players reporting a bug:

On clicking the verification link am getting an error message like:
The PL/SQL Challenge website is temporarily unavailable.
Please try again later or join the PL/SQL Challenge twitter group:
PLSQLChallenge
Sorry for the inconvenience.

Ugh. Well, the site is certainly available. So what's going on? Turns out the verification URL is missing the all-important "/pls/":

This:

http://www.plsqlchallenge.com/apex/f?p=QDB_PROD:34:::::P34_USER,P34_CODE:...

should be this:

http://www.plsqlchallenge.com/pls/apex/f?p=QDB_PROD:34:::::P34_USER,P34_CODE:...

Looked into the code and found:

   FUNCTION apex_website_url RETURN VARCHAR2
   IS
   BEGIN
      RETURN CASE 
              WHEN qdb_config.prod_state THEN APEX_UTIL.HOST_URL 
              ELSE website_url ()
              END 
              || '/apex/';
   END;

Now, a few things immediately come to mind: 
  • Huh? Why the CASE statement? Why distinguish between PROD and another instance of the app? Or to put it another way: please document your code!
  • Glad to have that built-in (APEX_UTIL.HOST_URL), but what does it do, really?
  • Does it need to be this complicated?
  • Oh, and where is it used?
Let's take that last item first. Before changing any code currently in production use (or even still in dev, but potentially in use across the app), it is so important to find out where that code is used, and how.

So I did that. I found two "hits" - one that is used to generate the verification URL and a second used in a program that sends out emails with the previous week's quiz results. 

But I took the analysis a little bit further and found that the second usage was "vestigial". Probably did use it at some point, but currently the function is called and assigned to a variable, but that variable is not used.

OK, add that to my clean-up list.

And, very nicely, I now know that this function is only used in the buggy scenario I am trying to fix. That gives me much more latitude to make changes.

So back to the apex_website_url. Since I am relying on a built-in, I suppose it might now hurt to actually look at the documentation. Where I find the following:


Well, woudja look at that....HOST_URL takes a parameter! And one value for that parameter is "SCRIPT" and if I use that, it automatically appends /pls/apex.

Confession: when I saw this I felt dumb. Or maybe it made me feel once again (happens pretty often) like the "closet amateur programmer" I believe myself to be (never studied computer science or algorithms or just about anything CS related in college). 

So I thought to myself: maybe, just maybe, I could change my function to nothing more than:

   FUNCTION apex_website_url RETURN VARCHAR2
   IS
   BEGIN
      RETURN APEX_UTIL.HOST_URL ('SCRIPT');  
   END;

I made the change. I tested the change. It worked!

THEN I moved it into production - and immediately tested again. Still worked. (ha! Who's the amateur now?). 

Another day, another problem (of our own making) solved.

Just one mystery remains: why did the verification email STOP working in mid September? Perhaps I will report back to you on that later. :-) 

This little episode was such a great reminder of some of the most important pieces of advice I can give to myself:

1. Read the documentation. Don't assume. Make sure you fully leverage all that the software vendor has done for you.

2. Keep your code as simple as possible. 

3. If for some reason you have to make it complicated, explain that complication with a comment.





Comments

  1. Hello Steven,

    Reading this post made me feel happy twice ( to just borrow your feeling from the previous post on this blog :):) )

    1. First of all, you are recommending to Read the Documentation .

    This is my "ALL THE TIMES" credo when I am doing anything,
    since I started working with a computer :) :)

    The downside is, however, that this is maybe the MAIN or even ONLY reason that
    up to this day I don't know APEX :( :(

    The documentation is HUGE, and I should READ IT before typing the first
    letter or opening the first page ....

    I would call myself "a person that knows APEX" when I will know about
    ALL those built-ins and their parameters :) :) :)

    Don't laugh at me ... this is about what I always did with Oracle Forms
    along all my career ... and it always worked wonders :):)

    By the way, I guess that the addition of that argument was probably
    related to some product version upgrade issue ...
    otherwise it would probably not have worked prior to this date also.

    2. Second, you recommend, AGAIN, using comments to document obscure code ...
    ... though, recently enough, in one of our Roundtable discussion threads I got the
    impression that you do favor self-documenting code ...

    Not adding comments can also be deliberate ... sometimes you want
    to trick others, for "good reasons" .... but, otherwise, for yourself and for the
    "good guys" around, it is better to have them included ...

    Cheers,
    Iudith Mentzel

    ReplyDelete
  2. Thanks, Iudith. I do like to keep my comments to a minimum, but that is ONLY justified if the code without comments is self-explanatory. Otherwise, yes, comment away! The problem with that guideline - and it is a problem that comes deep from within our brains and how we think - is that once we sort out what the code needs to be, everything seems so obvious. Comments seem superfluous. Perhaps the best way to do this is to set up a short-term feedback loop: Each day, first thing, look at the code you wrote yesterday and see if it's still "obvious". If not, write some comments before you COMPLETELY forget what the heck it is you wrote.

    ReplyDelete

Post a Comment

Popular posts from this blog

Table Functions, Part 1: Introduction and Exploration

Recommendations for unit testing PL/SQL programs

The future of Oracle PL/SQL: some thoughts on Sten Vesterli's thoughts