コレクションの無駄な処理
コレクションに対して、通常のプロジェクトであればコーディング規約で「コレクションはnullを返さないで空インスタンスを返す」という規約をよく見かける。一方で、この規約を初めて聞いたという人も多かった。その結果、無駄なnullチェックを良く見かける。
また、Java経験者にC#プロジェクトを手伝ってもらったときには件数チェックをしてからforループをする人がいた。コレクション操作については言語関係なく、あまり知られていない気がする。
サンプルコード
DBから取得してコンソールに取得した値を出力するコードを考える。
このコードの問題点はいくつかある。
1. コレクションにnullを想定している。
2. コレクションがnullの場合、「エラー」を出力しているが、例外の情報を握りつぶしている。
3. foreachを使用する前に件数チェックをしている。
※ちなみに、規約に「コレクションをnullにしてはいけない」規約があるプロジェクトにも関わらず、コレクションをnullにしていた。
internal void Main()
{
var items = GetDataFromDB();
// NG1 NULLチェックをしてからforeach
if (items != null)
{
foreach (var item in items)
{
Console.WriteLine(item);
}
}
// NG2 NULLチェックと件数チェックをしてからforeach
if (items != null && items.Count > 0)
{
foreach (var item in items)
{
Console.WriteLine(item);
}
}
// NG3 NULLがこないことは想定しているが、何故か件数チェック
if (items.Count > 0)
{
foreach (var item in items)
{
Console.WriteLine(item);
}
}
// NG4 listがnullならエラー
if (items == null)
{
Console.WriteLine("エラー");
}
}
/** DBからデータを取得する **/
private List<string> GetDataFromDB()
{
// DBからデータを取得して、エラーの場合はNULLを設定する。
// newの部分は、Daoからデータを取得している想定とする。
try
{
var list = new List<string>();
list.Add("1");
list.Add("2");
return list;
}
catch (Exception)
{
return null;
}
}
リファクタリング後
nullの想定をなくし、例外を戻り値に含めるようにした。
※例外処理をGetDataFromDB内で行うべきかは、ここでは考えていない。
internal void Main()
{
var result = GetDataFromDB();
var items = result.Items;
// nullチェックも件数チェックも不要
foreach (var item in items)
{
Console.WriteLine(item);
}
// 例外が発生している場合、その例外情報を出力する。
if (result.Exception !=null)
{
Console.WriteLine(result.Exception.ToString());
}
}
/** DBからデータを取得する **/
private Result GetDataFromDB()
{
// DBからデータを取得して、エラーの場合はNULLを設定する。
// new Listの部分は、Daoからデータを取得している想定とする。
try
{
var list = new List<string>();
list.Add("1");
list.Add("2");
return new Result(list, null);
}
catch (Exception ex)
{
return new Result(new List<string>(), ex);
}
}
/** 戻り値と例外処理を含めた処理結果クラス **/
private class Result
{
internal List<string> Items { get; private set; }
internal Exception Exception { get; private set; }
internal Result(List<string> items, Exception ex)
{
this.Items = items;
this.Exception = ex;
}
}
まとめ
コレクションをnullにしないことで、冗長なコードを排除できた。ただ、「foreachの前に件数チェックをする」という実装をする人には別の方法を考える必要がある。不思議なことに件数チェックをする場合としない場合があるので、その基準は不明である。「forやforeachの前で件数チェックをしてはいけない」という規約にしても直らないと思う。
また、nullで例外処理する人はコレクションでなくても同じコードを書くので、こちらも対処が難しい。この例外処理の問題はコレクションに限った問題ではない。
とりあえず、「コレクションをnullにしない」ことである程度問題は解決するので、実践してほしい。ただし、一律禁止しているわけではなく、どうしてもnullにしなければならない場合は、アーキテクトに説明して許可をもらうべきである。
サンプルコード(Java) 2017/3/24 追記
protected void main() {
List<String> items = getDataFromDB();
// NG1 NULLチェックをしてからfor
if (items != null) {
for (String item : items) {
System.out.println(item);
}
}
// NG2 NULLチェックと件数チェックをしてからfor
if (items != null && items.size() > 0) {
for (String item : items) {
System.out.println(item);
}
}
// NG3 NULLがこないことは想定しているが、何故か件数チェック
if (items.size() > 0) {
for (String item : items) {
System.out.println(item);
}
}
// NG4 listがnullならエラー
if (items == null) {
System.out.println("エラー");
}
}
private List<String> getDataFromDB() {
// DBからデータを取得して、エラーの場合はNULLを設定する。
// newの部分は、Daoからデータを取得している想定とする。
try {
List<String> list = new ArrayList<String>();
list.add("1");
list.add("2");
return list;
} catch (Exception e) {
return null;
}
}
リファクタリング後(Java)
protected void main() {
Result result = getDataFromDB();
List<String> items = result.getItems();
// nullチェックも件数チェックも不要
for (String item : items) {
System.out.println(item);
}
// 例外が発生している場合、その例外情報を出力する。
if (result.getException() != null) {
System.out.println(result.getException().toString());
}
}
/** DBからデータを取得する **/
private Result getDataFromDB() {
// DBからデータを取得して、エラーの場合はNULLを設定する。
// new Listの部分は、Daoからデータを取得している想定とする。
try {
List<String> list = new ArrayList<String>();
list.add("1");
list.add("2");
return new Result(list, null);
} catch (Exception e) {
return new Result(new ArrayList<String>(), e);
}
}
/** 戻り値と例外処理を含めた処理結果クラス **/
protected class Result {
private List<String> items;
private Exception exception;
protected Result(List<String> items, Exception exception) {
this.items = items;
this.exception = exception;
}
protected List<String> getItems() {
return items;
}
protected Exception getException() {
return exception;
}
}