関数の見通しをよくする(プログラム)
以下、メンテナンスしているコードの中にあったとある関数。(差し支えがないように少々定数名をいじっている)
Context という型から「ははあ Android か」とわかる方もいらっしゃいますよね?
public static String shurinkString(final Context context, final String str) {
String validateStr = str;
if (validateStr == null) {
validateStr = "";
} else {
validateStr = validateStr.replace("\r\n", "\n");
final int maxLength = context.getResources().getInteger(R.integer.text_max_length);
if (validateStr.length() > maxLength) {
validateStr = validateStr.substring(0, maxLength);
}
}
return validateStr;
}
関数の出口がひとつ、というのは微笑ましくも好ましい。
けれど、ちょっと長い。
ということで、僕だったらどうするかをお見せする。
* * *
まず関数の途中で出てくる Context (経由でリソース)から、文字列の最大長を引き出す部分を関数の外に放り出す。
public static String shurinkString(final String str, int maxLength) {
String validateStr = str;
if (validateStr == null) {
validateStr = "";
} else {
validateStr = validateStr.replace("\r\n", "\n");
if (validateStr.length() > maxLength) {
validateStr = validateStr.substring(0, maxLength);
}
}
return validateStr;
}
この呼び出し箇所で context を引数に渡しているのは明白。だから引数で context の代わりに context.getResources().getInteger(R.integer.text_max_length) を渡してもらうよう変更してもらう。
* * *
つぎは純粋に好みの問題ともいえるけれど、 validateStr の再代入をやめる。
手続き型言語は代入できることが大きな特徴だけれども、これはバグの温床だ。
処理の経過に沿って変化していくオブジェクトが、同じ変数でポイントされていると混乱する。とくにループ処理が長くなるような場合に。
public static String shurinkString(final String str, int maxLength) {
final String validateStr;
if (str == null) {
validateStr = "";
} else {
final String replaced = str.replace("\r\n", "\n");
if (replaced.length() > maxLength) {
validateStr = replaced.substring(0, maxLength);
}
}
return validateStr;
}
* * *
次は文字列長さの最大長設定部分。
以下の if 条件で分岐している箇所。もちろんこれで問題ないけれど……
if (replaced.length() > maxLength) {
こうする。
public static String shurinkString(final String str, int maxLength) {
final String validateStr;
if (str == null) {
validateStr = "";
} else {
final String replaced = str.replace("\r\n", "\n");
final int length = Math.min(replaced.length(), maxLength);
validateStr = replaced.substring(0, length);
}
return validateStr;
}
つまり、無条件に部分文字列(substring)を取得して返す、とした。
(わざわざ自分と同じ長さの部分文字列を返すことには無駄がある。けれど切り出す文字長が同じなら単にもとの文字列 this を返す処理系実装もある(たとえば Android の String 実装がそうだ))
* * *
さあ、 validateStr を消してみよう。
public static String shurinkString(final String string, int maxLength) {
if (string == null) {
return "";
} else {
final String replaced = string.replace("\r\n", "\n");
final int length = Math.min(replaced.length(), maxLength);
return replaced.substring(0, length);
}
}
* * *
いかがだろうか。
※構造化プログラミング手法の「出口はひとつ」というマントラを破っているけれど、この程度なら許してもらえないだろうか?
この記事が気に入ったらサポートをしてみませんか?