今回から2回にわたり、プログラムの品質確保のノウハウをまとめておきたいと思います。
これは新人に教える内容ではあるが、あえて教える人もおらず、文書としてまとめているところも見たことがないので、ここにまとめておこうと思います。
プログラムの品質は何で判断されるべきか?
プログラムの品質の良し悪しは何で決まるのでしょうか?
行数の少ないこと?
テストでバグが出ないこと?
品質を判断するのは先輩でも上司でもありません。成果物を受け取る人(使う人)です。
実行ファイルを使うのはユーザだがプログラムソースを受け取るのは保守運用担当者。
テストをしっかり行うことで、不具合のない状態にすることはできます。
しかし、ひとたびシステム障害が起きたり改造が必要になると、保守運用担当者がプログラムソースの修正を行うことになります。
プログラムソースが複雑だったり、難解だったり、不要なプログラムが含まれていると、担当者が混乱し、システム停止時間やコストが大きくなり、結果的にユーザビリティの低下につながります。
プログラム品質の確保に必要なものとしては、
- 単純であること
- 読みやすいこと
であると私は考えます。
今回は、どう言ったプログラムが読みにくく、どの様に書けば読みやすいのかを、いくつかの例とともに紹介したいと思います。
プログラム初心者にも対応するため常識的な内容も含まれますが、上級者の方でも一読して頂いて損はないと思います。
例1:紛らわしい変数名
例2:フラグの多様
例3:否定の否定
例4:条件文のネスト
例5:長い条件文
例6:関数のネスト
例7:上から下に流れないロジック
例8:スパゲッティ
例1:分かりにくい変数名
仕様
商品コード、単価、数量、税率、レート、円金額、外貨金額を受け取り、円金額が未入力の場合、
単価×数量×消費税(0.5)
により計算する。
外貨金額が未入力で、レートが入力されている場合は、
円金額×レート
により計算する。
ロジック
function setKingaku(p1, p2, p3, p4, p5, p6, p7 ){
if ( p6 == "" ){
return p2 * p3 * P4;
}
if ( p7 == "" && p5 != "" ){
return p6 * p5;
}
}
問題点
呼び出す側としては、各引数に何を入れればいいか分かりません。
内部のロジックを解析するときにも、引数に何が入ってくるかもわかりません。
業数の少ないロジックなら追えるかもしれないが、スクロールが発生する様なプログラムだと致命的です。
解説
初心者にありがちなプログラムです。
サンプルプログラムにこう言ったものが多いので、それを手本とするとこう言ったものになってしまうことがあります。
やはり、変数には意味のある名前を付けるようにしましょう。
また、変数に限らず、テーブル名、サービス名、ソースファイル名なども名前をつけるときには出来るだけ「名は体を示す」ことを意識してください。
改善ロジック
function setKingaku(shohin, tanka, suryo, zeiritsu, rate, kingaku, gaika){
if ( kingaku == "" ){
kingaku = tanka * suryo * zeiritsu;
}
if ( gaika == "" && rate != "" ){
gaika = kingaku * rate;
}
}
例2:フラグの多用
仕様
商品コード、単価、数量、税率、レート、円金額、外貨金額を受け取り、商品コードの上一桁が”A”の場合は、税額の入力を必須とし、円金額を以下の計算式で計算する。
単価×数量×消費税(0.5)
商品コードの上一桁が”D”の場合は、レートの入力を必須とし、外貨金額を以下の計算式で計算する。
単価×数量×レート
ロジック
function setKingaku(shohin, tanka, suryo, zeiritsu, rate, kingaku, gaika){
var enflg = false;
var dlflg = false;
var elseflg = false;
if ( shohin.substring(0,1) == "A" ) {
enflg = true;
}else if ( shohin.substring(0,1) == "D" ) {
dlflg = true;
}else{
elseflg = ture;
}
if ( enflg == true && zeiritsu == 0 ) {
throw new Error("税率の入力は必須です。");
}
if ( dlflg == true && rate == 0 ) {
throw new Error("レートの入力は必須です。");
}
if ( enflg == true ){
kingaku = tanka * suryo * zeiritsu;
}
if ( dlflg == true ){
gaika = tanka * suryo * rate;
}
}
問題点
複数のフラグを作成して特定条件でtrueを設定しています。
さらに設定したフラグの複合条件で処理を行っているため、入力(引数)に対して何が出力されるのかを読み解くことが困難な状態となっています。
この構造が多用されると、いわゆるスパゲティプログラムと言われてしまいます。
解説
フラグは出来るだけ使用しないことを心がけましょう。
慣れるまでは、1メソッドにつき1つ、多くても2つまでを目安に実装するようにします。
それ以上、必要となるようであればプログラム構造そのものに何らかの問題があることが多いので、一度、全体の構成を点検し、無駄な箇所などを整理するとこをお勧めします。
改善ロジック
function setKingaku(shohin, tanka, suryo, zeiritsu, rate, kingaku, gaika){
if ( shohin.substring(0,1) == "A" ) {
if ( zeiritsu == 0 ) {
throw new Error("税率の入力は必須です。");
}
kingaku = tanka * suryo * zeiritsu;
}else if ( shohin.substring(0,1) == "D" ) {
if ( rate == 0 ) {
throw new Error("レートの入力は必須です。");
}
gaika = tanka * suryo * rate;
}
}
例3:否定の否定
仕様
商品コード、単価、数量、税率、レート、円金額、外貨金額を受け取り、商品コードの上一桁が”D”でない場合は、税額の入力を必須とし、円金額を以下の計算式で計算する。
単価×数量×消費税(0.5)
上記以外の場合は、レートの入力を必須とし、外貨金額を以下の計算式で計算する。
単価×数量× ×レート
ロジック
function setKingaku(shohin, tanka, suryo, zeiritsu, rate, kingaku, gaika){
if ( ! (shohin.substring(0,1) != "D" ) ) {
if ( rate == 0 ) {
throw new Error("レートの入力は必須です。");
}
gaika = tanka * suryo * rate;
}else {
if ( zeiritsu == 0 ) {
throw new Error("税率の入力は必須です。");
}
kingaku = tanka * suryo * zeiritsu;
}
}
問題点
否定条件をさらに否定してしまっています。
その後でelseがあるため、またさらに否定となっています。
入力によりどの条件が実行されるかが追いにくく、バグの温床になりやすい状態です。
また、データパッチ対応などが必要なときに二次災害の原因となってしまいます。
解説
既存プログラムが否定で実装されており、その修正を行うときにありがちです。
改修要件が否定の場合でも、結果的に肯定である場合は肯定で実装するようにしましょう。
その結果、仕様と乖離が発生するようであれば、既存プログラムか仕様が間違っている可能性も考えられます。
要求から仕様、既存プログラムの確認を行って、乖離の原因を明確にすることが重要です。
例4:ネストした条件
仕様
aがtrue、かつ、bが0、かつ、cが1の場合は”OK”を表示。
上記以外は、以下のようなエラーを表示。
「○は×以外の設定はできません」
ロジック
function check(a, b, c){
if( a == true ) {
if( b == 0 ) {
if( c == 1 ) {
System.out.println( "OK" );
}else{
System.out.println( "cは1以外の設定はできません" );
}
}else{
System.out.println( "bは0以外の設定はできません" );
}
}else{
System.out.println( "aはtrue以外の設定はできません" );
}
}
問題点
if文が3階層に渡ってネスト(階層化すること)しています。
階層化した条件に対してelseがありますが、その処理がどういう条件で実行されるか分かりにくくなっています。
解説
慣れるまでは、ifに限らず、プログラムのネストは多くて2階層までにとどめた方がいいでしょう。
後に示す改善プログラムを見ても分かりますが、早く判断できる条件を先に判定し、その時点でメソッドやルーチンを終了してしまえば、分岐を少なくすることができます。
また、判定ロジックの後の処理はすべての条件をクリアした前提のロジックと読み取ることができるので、ソースの位置で実行される条件を見分けることができ、可読性が上がります。
人によっては、メソッドは必ず最後まで流れるようにした方がいいと言う人もいます。
確かに、メソッド終了時にクローズ処理などの必須処理を抜け漏れ防止のためにやる場合もありますが、そのために条件処理が複雑化するのは保守性を考えると避けるべきと思われます。
その場合は、try~catch~final、breakやgotoなどを利用し、プログラム構造を工夫するなどして恒久的に対応するようにしましょう。
改善ロジック
function check(a, b, c){
if( a != true ) {
System.out.println( "aはtrue以外の設定はできません" );
return;
}
if( b != 0 ) {
System.out.println( "bは0以外の設定はできません" );
return;
}
if( c != 1 ) {
System.out.println( "cは1以外の設定はできません" );
return;
}
System.out.println( "OK" );
}
例5:長い条件文
仕様
a+bが1、かつ、b+cが2、かつ、c+dが3、かつ、
d+eが4、かつ、e+fが5、かつ、f+gが6、かつ、
g+hが7の場合は”OK”を表示。
それ以外は”NG”を表示。
ロジック
function main(a, b, c, d, e, f, g, h){
if( a + b == 1 && b + c == 2 &&; c + d == 3
&& d + e == 4 && e + f == 5 && f + g == 6 && g + h == 7){
System.out.println( "OK" );
}else{
System.out.println( "NG" );
}
}
問題点
長く複数の条件が1つの判定文で判断しています。
このような判定文ではどの条件でTRUE(またはFALSE)となっているかわかりません。
ステップデバッグを行う場合もメモリ内の値を一つ一つ確認する必要があり、ログを設定する場合もすべての判定分の値をすべて出力し無ければなりません。
仕様変更により、条件に修正が必要になった場合も、すべてのパターンの再テストが必要になります。
解説
長い条件は扱いにくいですが、すべてをばらばらにするのも可読性が下がります。
一つの判定に含める条件は要件に合った組み合わせとしたほうがいいでしょう。
それでも長くなる場合は、判定メソッドを作成して、その中で個々の条件を分ける方法もあります。
改善ロジック
function chack(a, b, c, d, e, f, g, h){
if( a + b != 1 ) return false;
if( b + c != 2 ) return false;
if( c + d != 3 ) return false;
if( d + e != 4 ) return false;
if( e + f != 5 ) return false;
if( f + g != 6 ) return false;
if( g + h != 7 ) return false;
return true
}
function main(a, b, c, d, e, f, g, h){
if( chack(a, b, c, d, e, f, g, h) ){
System.out.println( "OK" );
}else{
System.out.println( "NG" );
}
}