前回のプログラムは改良したとは言え、まだまだ突っ込みどころ満載です。そういう訳なので、今回も更に指摘して行きます。
今回はaCollection
の要素を順に取り出す処理の実現方法に注目します。前回のプログラムの何が良くないのか分かりますか?
前回のプログラム
import java.util.Iterator;
import java.util.Collection;
public class CollectionUtility
{
public static String toString(Collection aCollection)
{
Object tBuffer;
String tString = "";
Iterator tIterator = aCollection.iterator();
for(int i = 0; i < aCollection.size(); i++)
{
if(i == 0)
{
tBuffer = tIterator.next();
tString += tBuffer;
}
else
{
tBuffer = tIterator.next();
tString += ", " + tBuffer;
}
}
return tString;
}
}
そのプログラムはこれ以上ない程に明快か?
今回指摘するのはプログラムの明快さです。このプログラムはまだまだ明快にする余地があります。そういう余地を残さないようにするべきです。ほとんどの場合、これ以上ない程に明快にして無駄なことはないからです。
まず、このプログラムの明快さを低下させているのはコレクションの終端を判定するためにCollection
のメソッドsize()
を使っている部分です。要素を順に取り出すためにインターフェイスIterator
を使う場合、通常はメソッドhasNext()
で終端を判定します。しかし、このプログラムはそうなっていません。そのため、読み手は何かの意図があるかもしれないと考えてしまうかもしれません。読み手を迷わせない、無駄な時間を使わせないようにプログラムを記述するべきです。
明快さの他に性能的観点からもメソッドhasNext()
を使うべきです。このメソッドが所属するインターフェイスIterator
は要素を順に取り出すことを第1の目的に設計されているため、今回の用途に対して適切な性能を発揮する可能性が高くなります。それに対してメソッドsize()
が所属するインターフェイスCollection
は要素を順に取り出すことを第1の目的に設計されている訳ではないため、今回の用途に対して適切な性能を発揮する可能性が低くなります。例えば、インターフェイスCollection
の実装として素朴なリンクリストを想定した場合、要素数を求めるメソッドsize()
の計算量は要素数に比例します。引数aCollection
にリンクリストが代入された場合、メソッドtoString(Collection)
の計算量は引数aCollection
の要素数の2乗に比例します。このような性能劣化を防ぐには最も目的にあったメソッドを利用して実装する必要があります。
以上のことを踏まえた上で、Tさんに修正してもらいました。次のプログラムは修正後のもの(を説明用に修正したもの)です。
プログラム(hasNext()
を使用)
import java.util.Iterator;
import java.util.Collection;
public class CollectionUtility
{
public static String toString(Collection aCollection)
{
Object tBuffer;
String tString = "";
Iterator tIterator = aCollection.iterator();
for(int i = 0; tIterator.hasNext(); i++)
{
if(i == 0)
{
tBuffer = tIterator.next();
tString += tBuffer;
}
else
{
tBuffer = tIterator.next();
tString += ", " + tBuffer;
}
}
return tString;
}
}
次に、このプログラムの明快さを低下させているのはカウンタ変数i
を使って最初の要素とそれ以降の要素を区別している部分です。よく考えれば当然ですが、カウンタ変数i
は必要ありません。なぜならば、for
の1回目のループ処理をfor
の前にくくりだせば最初の要素とそれ以降の要素を処理する部分を区別できるからです。
以上のことを踏まえて、Tさんに修正してもらいました。今回は少し複雑な修正となるので、これから示す手順で行ってもらいました。
まずは、for
をwhile
に修正してもらいました。この修正は次の2つのプログラムの意味が同じである性質を利用しています。
for(AAA; BBB; CCC)
{
DDD
}
AAA;
while(BBB)
{
DDD
CCC;
}
次が、for
をwhile
に修正したプログラムです。
プログラム(for
⇒ while
)
import java.util.Iterator;
import java.util.Collection;
public class CollectionUtility
{
public static String toString(Collection aCollection)
{
Object tBuffer;
String tString = "";
Iterator tIterator = aCollection.iterator();
int i = 0;
while(tIterator.hasNext())
{
if(i == 0)
{
tBuffer = tIterator.next();
tString += tBuffer;
}
else
{
tBuffer = tIterator.next();
tString += ", " + tBuffer;
}
i++;
}
return tString;
}
}
次に、while
の1回目のループ処理をくくりだしてもらいました。この修正は次の2つのプログラムの意味が同じである性質を利用します。
while(AAA)
{
BBB
}
if(AAA)
{
BBB
while(AAA)
{
BBB
}
}
次が、for
をwhile
に修正したプログラムです。
プログラム(1回目のループ処理のくくりだし)
import java.util.Iterator;
import java.util.Collection;
public class CollectionUtility
{
public static String toString(Collection aCollection)
{
Object tBuffer;
String tString = "";
Iterator tIterator = aCollection.iterator();
int i = 0;
if(tIterator.hasNext())
{
if(i == 0)
{
tBuffer = tIterator.next();
tString += tBuffer;
}
else
{
tBuffer = tIterator.next();
tString += ", " + tBuffer;
}
i++;
while(tIterator.hasNext())
{
if(i == 0)
{
tBuffer = tIterator.next();
tString += tBuffer;
}
else
{
tBuffer = tIterator.next();
tString += ", " + tBuffer;
}
i++;
}
}
return tString;
}
}
ここで、カウンタ変数i
に注目すると、1つ目のif
はthenの処理しか実行されず、2つ目のif
はelseの処理しか実行されないことが分かります。このことを考慮して無駄な処理を削除したプログラムが次です。
プログラム(無駄処理除去)
import java.util.Iterator;
import java.util.Collection;
public class CollectionUtility
{
public static String toString(Collection aCollection)
{
Object tBuffer;
String tString = "";
Iterator tIterator = aCollection.iterator();
int i = 0;
if(tIterator.hasNext())
{
tBuffer = tIterator.next();
tString += tBuffer;
i++;
while(tIterator.hasNext())
{
tBuffer = tIterator.next();
tString += ", " + tBuffer;
i++;
}
}
return tString;
}
}
さらに、カウンタ変数i
に注目すると、この変数の値が結果になんの貢献もしていないことが分かります。また、変数tBuffer
に注目すると、この変数に値を代入した次の行で使っているだけだと分かります。このことを考慮して無駄な変数を削除したプログラムが次です。
プログラム(無駄変数除去)
import java.util.Iterator;
import java.util.Collection;
public class CollectionUtility
{
public static String toString(Collection aCollection)
{
String tString = "";
Iterator tIterator = aCollection.iterator();
if(tIterator.hasNext())
{
tString += tIterator.next();
while(tIterator.hasNext())
tString += ", " + tIterator.next();
}
return tString;
}
}
どうですか?かなり明快になったと思いませんか?当初のプログラムと比べて半分程度の行数になっています。このくらい明快なら読み手を迷わせたり無駄な時間を使わせたりしないですむと思います。
さらに重要なのは今回の修正方法です。この方法は公式に当てはめて方程式を式変形するのに似ています。疑いようがないほと単純な公式に当てはめるという機械的作業を画面上で行うだけなので頭で推論する必要がほとんどありません。そのため修正過程に飛躍が入り込む余地が少なく安全です。
この修正方法は今回のような場面や複雑な条件処理などを単純化するのに非常に役立つので覚えておいて損はありません。
ここまで修正してかなり良いプログラムになりました。しかし、まだ改良する余地はあります。ヒントは「メソッドtoString(Collection)
がどのように使われるのか?」です。
2件のコメント
squld · 2007-06-08 01:17
> まだ改良する余地はあります。
> ヒントは「メソッドtoString(Collection)がどのように使われるのか?」です。
うーん。
先生、カンマ区切りはイヤです!w
来栖川ブログ » プログラムは語る: toString編3 · 2009-03-10 15:58
[…] 前回の最後に『しかし、まだ改良する余地はあります。ヒントは「メソッドtoString(Collection)がどのように使われるのか?」です。』と謎を残しておきました。今回はその謎解きです。性能に注目して前回のプログラムを見てください。 […]
現在コメントは受け付けていません。