件のソースではある設定値を、配列(BCD)・整数・構造体の3通りの方法で格納している。
なんでこんなことになっているかと言うと、これまた回路の都合を引きずっているから。
配列には、値をBCDにしてから桁並び順を反転したものが格納される。つまり 1234 という値は { 0, 0, 0, 0, 0, 4, 3, 2, 1 } という形で格納される(配列長は9で固定)。配列の形にしておけば数字をLCDに表示したり、桁ごとにインクリメントやデクリメントするのに都合がよい。
整数型には回路ICに送信するデータが格納される。格納する値はある計算式を使って上記の配列から算出される。
構造体は関数から値と関数の結果をまとめて返すために用意されている。普通のやり方なら値はポインタ引数で返し、関数の結果を戻り値にする。そうすればこんな構造体は不要になる。
性質が悪いことに、ソースのあちこちでその3通つの型の間で値を変換している。たとえば次の例のように、値を「インクリメントするときには配列」に変換し、「大小比較をするときには整数型」に変換し、「関数から値を戻すときには構造体に変換」している。
#define ARRAY_SIZE 9 unsinged char aMax[ARRAY_SIZE]; unsinged char aMin[ARRAY_SIZE]; unsinged char aVal[ARRAY_SIZE]; ... typedef struct { unsigned char array[ARRAY_SIZE]; //関数で計算した値を格納 int retval; //関数の実行結果を格納 } st_Struct; st_Struct foo(void) { int i; st_Struct sVal; for (i = 0; i < ARRAY_SIZE; ++i) { aVal[i] = aMin[i]; //ループ変数の初期化 } int nMax = IntToArray(aMax); //ループの終値を整数に変換 int nVal = IntToArray(aMin); //ループ変数の初期値を整数に変換 while (nVal < nMax) { IncrementArray(aVal); //値のインクリメントは配列で nVal = ArrayToInt(aVal); //while文での値の比較のために整数に変換 ... if (some_condition()) { for (i = 0; i < ARRAY_SIZE; ++i) { sVal.array[i] = aVal[i]; //戻り値の構造体に値を格納 } sVal.retval = 0xFFFFF; //戻り値の構造体に実行結果を格納 return sVal; } } for (i = 0; i < ARRAY_SIZE; ++i) { sVal.array[i] = aVal[i]; //戻り値の構造体に値を格納 } sVal.retval = 0x00000; //戻り値の構造体に実行結果を格納 return sVal; }
実際には型が推測できない変数名が付けられているので非常に読みにくい。
上のコードは無駄に複雑になっているが意図するところは単純で、int型で書けば次のようになる。
int nMax, nMin, nVal; ... int foo(int* pResult) { int i, nVal; for (nVal = nMin; nVal < nMax; ++nVal) ... if (some_condition()) { *pResult = nVal; //ポインタ経由で値を返す return SUCCESS; //成功 } } return FAILED; //失敗 }
今回の場合、32bit整数より大きい値を扱うので何らかの構造体を用意せざるを得ないが、インクリメントや比較のたびに型を変換するのではなく、その構造体のままインクリメントや比較を行う関数を用意すべきだろう。