2018 03 04

PL/SQLのリファクタリングの1

俺の見ている範囲でありがちなのは、昨日も挙げた通りのパターン。

たぶん最初の一歩を踏み誤ったせいでこうなったんだよな。 最初の一歩というのは、処理の追加を既存モジュールの中に入れるのか別のモジュールを作るのか、という判断。 最初の改造時に 「面倒だからここに入れちゃえ」 とやったのだ。 後に続く人たちはそんなソースを見て、きっと少なくとも一度は綺麗にする事を考えるのだが、いろんな制約との葛藤の中で 「まあ同じでいいか」 とやってしまう。 その積み重ねによる肥大化複雑化。

とはいえ、時間も金も無い時に 「後の事を考えて、ここは赤字になっても綺麗にコードを書く」 なんてやったところで、そいつの評価は 「赤字を出した」 で終わりだし、やろうって気にならない気持ちも分かるけどさ。

まあいろいろあったんだろうが、それはそれとして、ここでは 「一つのモジュールの中にいくつも処理を追加している」 というパターンをどう綺麗にするかを考える。 その追加された処理が、それぞれ独立しているという前提で。 なんかすっごい御都合主義に聞こえるが、実際そんなのがあるからね。

さっきまで昔行った展覧会の図録を眺めていたので、本をネタにサンプルを。 実際に本がどう管理されているのかは知らないが、こんな処理をやっているとする。

とりあえず、締め日は毎月15日としておこう。 コードはこんな感じ。

create or replace package 月次処理 is PKG_NM constant varchar2(30) := '月次処理' ; function 当月分コピー return number ; end ; / create or replace package body 月次処理 is function 当月分コピー return number is FNC_NM constant varchar2(30) := '当月分コピー' ; FIN_DT constant number := 15 ; v_bgn date ; v_end date ; v_cnt number := 0 ; v_errm varchar2(512) ; begin insert into バッチログ ( 日時 , 処理モジュール , 処理状況 , 処理件数 , 補足事項 ) values ( systimestamp , PKG_NM || '.' || FNC_NM , '処理開始' , 0 , null ) ; commit ; -- v_bgn := add_months( trunc( sysdate, 'MONTH' ), -1 ) + FIN_DT ; -- 先月16日00:00:00 v_end := add_months( v_bgn, 1 ) - 1 / ( 24 * 60 * 60 ) ; -- 当月15日23:59:59 -- insert into 月次納品 ( ISBN , 納品先ID , 工場出荷日 , 納品予定日 , 納品実績日 , 価格 ) select ISBN , 納品先ID , 工場出荷日 , 納品予定日 , 納品実績日 , 価格 from 日次納品 where 納品実績日 between v_bgn and v_end ; v_cnt := v_cnt + sql%rowcount ; -- insert into 月次返品 ( ISBN , 納品先ID , 返品予定日 , 返品実績日 , 払戻価格 , 負担割合 , 返品理由 ) select ISBN , 納品先ID , 返品予定日 , 返品実績日 , 払戻価格 , 負担割合 , 返品理由 from 日次返品 where 返品実績日 between v_bgn and v_end ; v_cnt := v_cnt + sql%rowcount ; -- insert into バッチログ ( 日時 , 処理モジュール , 処理状況 , 処理件数 , 補足事項 ) values ( systimestamp , PKG_NM || '.' || FNC_NM , '正常終了' , v_cnt , null ) ; -- commit ; return 0 ; exception when others then rollback ; dbms_output.put_line( dbms_utility.format_error_stack() ); dbms_output.put_line( dbms_utility.format_error_backtrace() ); -- v_errm := sqlerrm ; insert into バッチログ ( 日時 , 処理モジュール , 処理状況 , 処理件数 , 補足事項 ) values ( systimestamp , PKG_NM || '.' || FNC_NM , '異常終了' , 0 , v_errm ) ; commit ; return 1 ; end ; end ; /

テーブル名とか列名とかは思いつき。 ここで問題にしているのはプログラムの構造であって性能ではないので、キー構成とかインデックスとか無視。 締め日も、ちゃんとやるならカレンダーや定数マスターに定義しておくことになるのだろうが、これまた今回の関心外として、とりあえず定数として内部定義。

ソースコードの背景にある脳内ストーリーはこんな感じ。

最初は納品だけだった。 その後に返品も扱うことになった。

納品と返品は、テーブルとしては独立しているが、データの意味合いからは関連が深そう。 だったら同じタイミングで扱った方がいいよね。 しかもきっと納品側がベースになるだろうから、納品側が失敗したら処理を続ける意味無いよね。

なんて言い訳をしながら、返品側の処理を追加した。

サンプルなので小さいけど、実際の困った子は、これが数千行だからね。 しかも千の桁の値が後半とか。 エディタで開いたときの待ち時間。 スクロールバーの掴む部分がどんどん小さくなる様子。 その段階でもう嫌な予感しかしないのに、なんとか開いてみれば、古い処理がコメントアウトされて残っていて有効部分が細切れだったり、途中でインデントがずれていたりするのだ。 読みにくいとかいうレベルじゃない。 はっきりとした悪意を感じるレベル。 てめえらの血は何色だ。

ということで、これを(少なくとも俺が)すっきりする形に改善するのだが、テスト用のテーブルやデータの準備をしていたら深夜になってしまったので、続きはまた明日。