Thursday, March 2, 2017

Here's a great way to put an infinite loop into your code.


Isn't that something you always wanted to do?

:-)

No, it's not. And I did that yesterday in my dev environment (well, of course, such a thing could never make it to production!). It is an enormous pain. 

You press the Run button. 

The process doesn't return in the usual 2 seconds.

You think back over the changes you just made and feel sweat break out on your forehead. Because you can see right away what you did and.....oh, how could I be so stupid?



Well, not stupid. Just in too much of a hurry. And careless. And over-confident. And thinking about too many things at once. You know, the sorts of things, "gurus" do all the time as a way of maintaining their high level of excellent to show to the world. :-(

So yes, I did this yesterday, and I thought I'd share with you my mistake to hopefully help you avoid doing the same thing in the future.

I am writing a program to automatically generate workouts for the Oracle Dev Gym (which will soon take over from the PL/SQL Challenge as an "expertise through exercise" learning platform).

I am relying heavily on collections (PL/SQL arrays). Now, I don't know about all of you, but I often go through several iterations of the design of those collections:
  • Use a collection of IDs? No, a collection of records. 
  • Use an integer indexed array? Hmmm, no, wait, maybe it should be string indexed...?
  • Oh, here's a great opportunity to use a nested collection!
And so on. It's all great fun, and the end result is usually less code and a cleaner algorithm. But along the way, it's kind of messy.

In this particular instance of an infinite loop, I had started out with a nested table to hold comma-delimited lists of quiz IDs. This nested table was densely-filled and so my loop looked like this:

PROCEDURE create_workouts_for_sets (
   resource_in    IN ov.ov_resources_external%ROWTYPE,
   quiz_sets_in   IN quiz_sets_t)
IS
BEGIN
   FOR indx IN 1 .. quiz_sets_in.COUNT
   LOOP
      create_workout;
      parse_list (quiz_sets_in(indx), l_quizzes);
      load_workout_actitivies (l_quizzes);
   END LOOP;
END;

Except that I didn't actually create those nested subprograms (create_workout, etc.). Instead, the body of the loop contained all the logic and extended for 100+ lines of code (thereby violating one of my personal favorite best practices: keep your executable sections tiny and highly readable). This point will become important in a moment.

OK, so as I built more of the algorithm, I realized that I needed to make sure I wasn't generating multiple workouts with the same list of quizzes. How to check for duplication? I suppose I could compare those comma-delimited lists....but, wait! Why I am creating a comma-delimited list to begin with? Why not have a collection of the selected quizzes?

And, another brainstorm: why not use that comma-delimited list instead as the index for the array? Then it is transparently easy to tell if there is duplication: does an element exist at that location in my now-string indexed array?

That sounds like fun! So I switched to a collection of records indexed by string (associative array):

SUBTYPE quiz_list_index_t IS VARCHAR2 (4000);

TYPE quiz_set_rt IS RECORD
   (
      maximum_time    INTEGER,
      difficulty_id   INTEGER,
      quizzes         numbers_nt
   );

TYPE quiz_sets_t IS TABLE OF quiz_set_rt
      INDEX BY quiz_list_index_t;

Then I changed the loop as follows:

PROCEDURE create_workouts_for_sets (
   resource_in    IN ov.ov_resources_external%ROWTYPE,
   quiz_sets_in   IN quiz_sets_t)
IS
   l_index quiz_list_index_t := quiz_sets_in.FIRST;
BEGIN
   WHILE l_index IS NOT NULL 
   LOOP
      create_workout;
      parse_list (quiz_sets_in(indx), l_quizzes);
      load_workout_actitivies (l_quizzes);
   END LOOP;
END;

And then after making a whole bunch more edits, and getting the package to compile, I decided to try it out.

I executed the parent procedure of create_workouts_for_sets....and it disappeared into NeverLand, never to return. Can you see the problem? Hopefully, it was instantly clear for you since the executable section above is so small:
I never change the value of l_index. Now that, dear friends, is one tight little infinite loop, right there.
In my program, however, because I had not yet refactored the 120-line body into nested subprograms, the END LOOP was "off the page", out of view, and therefore out of thought.

I needed to move on to the next-defined element in the collection, as follows:

PROCEDURE create_workouts_for_sets (
   resource_in    IN ov.ov_resources_external%ROWTYPE,
   quiz_sets_in   IN quiz_sets_t)
IS
   l_index quiz_list_index_t := quiz_sets_in.FIRST;
BEGIN
   WHILE l_index IS NOT NULL 
   LOOP
      create_workout;
      parse_list (quiz_sets_in(indx), l_quizzes);
      load_workout_actitivies (l_quizzes);
      l_index := quiz_sets_in.NEXT (l_index);
   END LOOP;
END;

You saw that, right? If not, you see it now, correct?

And that, readers, brings me to the point of this post:
When you are switching from dense to sparse collections, you will also likely need to shift from a numeric for loop to a simple or while loop, to iterate through the collection. 
When you make that change, you must not only change the header of the loop, but also add the necessary code to cause loop termination.
Or as is often said in programming circles: D'oh!

2 comments:

  1. Curious Steven, I don't see the declaration of the variable "l_quizzes" in your code

    ReplyDelete
    Replies
    1. Yes, sorry, it's kind of like pseudo-code. Assume that l_quizzes was declared at a higher scope. Which means I am referencing it "out of scope". Which makes me a bad boy. But l_quizzes is not germane to the point of the post, so I will keep it like it is.

      Delete