Improve code in terms of better approach and flaws [closed]

Question

Closed. This question does not meet Stack Overflow guidelines. It is not currently accepting answers.

---

Want to improve this question? Update the question so it's on-topic for Stack Overflow.

Closed 7 years ago.

Improve this question

I have the following PL/SQL code which I am using for calling an API (provided by client) in order to adjust accounts.

I would like to improve my code if there any short comings or better way or approach in writing this block.

P.S. Unfortunately our client is still using Oracle 8i for their product.

PLSQL Code

DECLARE
    v_account_id account_adjustments.account_id%TYPE;
    v_adj_no     account_adjustments.adj_no%TYPE;
    v_old_qty    product_warehouse.new_qty%TYPE;
    v_new_qty    product_warehouse.new_qty%TYPE;
BEGIN
    v_old_qty := NULL;
    v_new_qty := NULL;

    FOR acnt IN (SELECT *
                 FROM   temp_table a) LOOP
        SELECT a.new_qty
        INTO   v_old_qty
        FROM   product_warehouse a
        WHERE  ( a.product_no = acnt.product_no );

        Adjust_accounts (in_service_product_no => acnt.product_no,
                         in_service_qty => acnt.qty,
                         out_account_id => v_account_id,
                         out_adj_no => v_adj_no);

        SELECT b.new_qty
        INTO   v_new_qty
        FROM   product_warehouse b
        WHERE  ( b.product_no = acnt.product_no );

        INSERT INTO account_year_todate
                    (product_no,
                     account_id,
                     adj_no,
                     qty_bad,
                     qty_warehouse,
                     qty_factory)
        VALUES      ( acnt.product_no,
                     v_account_id,
                     v_adj_no,
                     acnt.qty,
                     v_old_qty,
                     v_new_qty );
    END LOOP;
EXCEPTION
    WHEN OTHERS THEN
      dbms_output.Put_line ('exceptions '
                            || SQLERRM);

      ROLLBACK;
END; 

Answer 1

Some things to change:

• Replace SELECT * in the For loop cursor with the columns that you actually need.
• Your exception block is what I call a silent killer. That's a debugging nightmare because it does nothing except for suppress exceptions because you aren't logging it, you're not re-raising it - instead just redirecting all exceptions via put_line into a buffer, which isn't visible at all, unless you're running the program from a IDE which shows buffer output always, or via SQL*Plus. Change it to a proper logging system.
• I have no idea what Adjust_accounts does - and you could probably reduce those 2 select statements and have the amounts returned using the stored procedure you're calling?