CodeSOD: The Apex of Development
David S writes: "I'm undertaking a refactor and facelift of an Oracle APEX application."
That, already, is the real WTF. Oracle Application Express, or APEX (formerly ApEx, formerly HTML DB) is Oracle's offering in the low-code business application space. Using a WYSISYG designer, you build pages and bind them to SQL queries, stored procedures, etc., allowing users with little to no programming experience to design data driven applications.
Like all such tools: it works fine for the very simple tasks, but once you try and model real-world applications in it, everything falls apart. Some of this is just the nature of low-code tools. Some of this is because much of Oracle APEX is implemented in Oracle's PL/SQL database language. Some of this is because Oracle keeps bolting features onto it, hoping that it finally gets the traction they want for it.
Which, on the scope of traction, you can see the collection of applications folks want to admit to having built in APEX here, which includes "Target Executive Search" (a job site for finding executives) and "My Karaoke". APEX has range. There are dozens of other such sites, including Built with Apex itself..
But none of that is David's problem. David inherited this PL/SQL code which is invoked from an APEX page.
DECLAREv_func boolean;rowcheck number;watcher_id number;--find every watcher in table for change_rec CURSOR c1 IS SELECT * FROM CHR_WATCHER WHERE CHRW_CHANGE_NO = :P2_CHR_CHANGE_NO AND CHRW_ACTIVE = 1;BEGIN--count number of watchers for the recordSELECT COUNT(*) INTO rowcheck FROM CHR_WATCHER WHERE CHRW_CHANGE_NO = :P2_CHR_CHANGE_NO AND CHRW_ACTIVE = 1;--if there is only one watcher for recordIF rowcheck = 1 THEN SELECT CHRW_PER_ID INTO watcher_id FROM CHR_WATCHER C WHERE c.CHRW_CHANGE_NO = :P2_CHR_CHANGE_NO AND CHRW_ACTIVE = 1;v_func:=send_email_watch (watcher_id, :P2_CHR_AUTH2, :P2_CHR_PERSON_RESP, :P2_CHR_CHANGE_DESC,:P2_CHR_CHANGE_NO,:P2_CHR_CHANGE_DATE);--if there is more than one then loopELSIF rowcheck > 1 THEN FOR record IN c1 LOOP--DBMS_OUTPUT.PUT_LINE(record.CHRW_PER_ID);v_func:=send_email_watch (record.chrw_per_id, :P2_CHR_AUTH2, :P2_CHR_PERSON_RESP, :P2_CHR_CHANGE_DESC,:P2_CHR_CHANGE_NO,:P2_CHR_CHANGE_DATE);END LOOP;END IF;END;
First off, as is standard for PL/SQL, we need to declare all our variables in a block at the top. These variables include a cursor, which is Oracle's main way of interacting with records.
At the top of the function, we use a SELECT INTO which is the other common way of interacting with data in the database. The query in this case is exactly the same as the cursor, except it's a count of the records.
Then we have our logic: if the number of rows is 1, run the query again to populate a variable, and call the send_email_watch function with the results. If there are more than one rows, use the cursor and loop across the results, calling the same function.
Why the branch? It's a mystery. My suspicion is that the code was originally written with the assumption there would only ever be one row handled by this code. Someone requested that it support multiple rows, so boom: we add a branch. This solution shows a radical lack of understanding regarding loops though, since a loop that only executes one iteration is still a loop.
"To be fair," David writes, "it runs fine." That doesn't mean the code isn't getting refactored, but it does at least do its job, which is something, I suppose.
[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!