EarOwlの日記: [C言語] ちょっとした愚痴 1
文字/文字列として使用する型を、 (typedef signed char my_char_t みたいに) signed/unsigned を指定して typedef しないで欲しい。
某組込向けライブラリがこれをやっていて、しかも char 型と符号の有無が異なるせいで、そのライブラリの API を呼び出す箇所で明示的に型変換しないと Warning が出る。
EarOwlさんのトモダチの日記、みんなの日記も見てね。 Idle.slashdot.jpは、あなたの人生において完全な時間の浪費です。見るなよ、見るなよ。
文字/文字列として使用する型を、 (typedef signed char my_char_t みたいに) signed/unsigned を指定して typedef しないで欲しい。
某組込向けライブラリがこれをやっていて、しかも char 型と符号の有無が異なるせいで、そのライブラリの API を呼び出す箇所で明示的に型変換しないと Warning が出る。
例として、気温・湿度・気圧・風向・風速の 5つの int 型のデータを 100回分保存するバッファを考える。
よく見かけるのが、
int weather_data[100][5];
という形で確保して、
weather_data[n][0] = temp;
weather_data[n][1] = hum;
…
という風に保存するという書き方。 (もっと酷いものだと int weather_data[5 * 100]; なんていうのもあるが…。)
これだと、『1回分のデータ配列中の何番目にどのデータ (気温・湿度…) が保存されているか』ということがソースコード上で明確でなくなってしまう。
配列のインデックスとデータを対応付ける #define を
#define TEMP_OFFSET 0
#define HUM_OFFSET 1
…
のように定義し、それを使用して
weather_data[n][TEMP_OFFSET] = temp;
weather_data[n][HUM_OFFSET ] = hum;
…
のようにすることで回避は出来る。 (しかし、それをちゃんと出来ていないソースコードもまた多い。)
もっと良い方法は、
typedef struct {
int temperature;
int humidity;
int pressure;
int wind_direction;
int wind_speed;
} weather_data_t;
weather_data_t weather_data[100];
のように構造体を用いることである。
こうすることで、
weather_data[n].temperature = temp;
weather_data[n].humidity = hum;
…
のように配列のインデックスではなく要素名を用いることになり、要素名からデータの内容も明確になる。
weather_data[n].temperature = hum;
のように誤った要素に保存するコードを書いてしまう可能性は
weather_data[n][0] = hum;
のように配列のインデックスに誤った値を直値で指定してしまう可能性と比較すれば極端に低く、よりバグを生みにくいコードであると言える。
某社から受け取ったソースコード。
main 関数では初期化処理のみを行い、主要な処理は main 関数を抜けた後に別の関数を呼ぶということを素でやっている…。
そういうのはネタだけにして欲しい…。
私の前に開発を担当していた人によると、そのソースコードは『後で直すつもりで雑な書き方のままの箇所が多く残っている』ということだったのですが、個人的にはこういうコーディングの進め方にはあまり賛同できない。
当初からきれいに書くことを意識していても、規模が大きくなるにつれて汚い部分が出てきてしまうことはままある。
まして最初から雑な書き方で進めていったら、後からの修正では雑な部分を取り除ききれなかったり、修正にかかる負担がかえって大きくなったりしてしまう。
さらに、とりあえず動作するコードを早急に書かなければならない場合でも、雑な分早く書くことが出来るかというと、あまりそうは思えない。むしろきれいに書くことを心がけた方が、見通しが良くなり早く完成させられるように思われる。
その1 : GUI 編
ある数値の表示にテキストボックスを使っていて…その数値を保存するときには…テキストボックスからテキストを取得して…文字列→数値変換をして…保存している…
さらに…そのテキストボックスはデータの表示に使うだけで…ユーザーがデータを入力する必要は無いもの…それなのに入力が禁止されていない…
その2 : DB (Access) 編
mdb ファイルが複数あって…複数の mdb ファイルにまたがるいくつかのテーブルで…オートナンバーで振られる ID の値が…一致する前提になっている…
つまり…DB にデータを追加する際は…作業者は ID が一致するように注意を払わなければならない…
殴り書きの設計書ではいくら内容が正しくても認めてもらえないのと同じように、殴り書きのソースコードもいくら動作が正しくても完成したと見なしちゃダメだよなぁ…。
前回の日記はプロジェクトの進め方等にかかわる部分でしたが、今度はプログラムの中身について。
そもそもこのプロジェクトの目的は、今までバラバラに作成されてきた、各種のターゲットに対する似たような処理を行うツール一式を、一つにまとめた上で、可能なところを自動化して効率を上げることと、今後の分析等のためにデータを蓄積することらしい。
ということで、本来ならば各種のターゲットに対する処理の共通点・相違点を一通り調査・理解し、メンテナンス性を考慮した設計が求められるところのはずが、どうやら特定のターゲットに対し動作するものを早急に要求されたこともあって、設計が不十分なままそのターゲットに対応する部分だけ動作するものを作ったっぽい。
さらに、データの蓄積のために Access を使っているのだけれど、データベースの使い方も、今までデータベース関連の開発などしたことがない私の目から見てもメチャクチャ。データ構造は『DB を Excel か何かと勘違いしてるんじゃ?』という感じだし、フィールド名も直観的でないし、不用意にデータを上書きしているところがあるし、そもそもDB以前のデータ分類整理の仕方がなっていないし。
という状況で引き継いだ上、スケジュールや費用を考えると大幅な修正を行うわけにもいかず、追加開発分も結局やっつけにやっつけを重ねるようなやり方でなんとか開発を進めている状況。
もし私がこのプログラムを分析して実用に堪えるかどうかを調べるだけの立場なら、間違いなく『こんなものは捨てて新規にちゃんとした設計で作り直すべき』と判断するところなんですが。
さらにまずいのは、お客さんはそんな内部設計のまずさまでは見ていなくて、とりあえずここまではそれなりにちゃんと開発できているように見えているであろうこと。
この状況でこれらの問題を取りざたしても、こちらの開発上の問題として、下手をすれば無償での対応を要求されかねない。そうなれば私が対応させられることも目に見えているわけで。 (より良い設計で新しく作り直せるのは、技術者的にある意味魅力的でもあるけれど。)
結局、追加開発が降ってくるたびにやっつけでの対応を繰り返しつつ、なんとか上手いことこの案件からフェードアウトしていくしかないような。
今やっているプロジェクトがそれはもう酷すぎてちょっと愚痴る。
受託開発の案件なんだけれど、そもそも要求仕様があまりにも不明瞭。
重要な部分の処理が『これまで使っていたソースコード/ツールがあるのでそれを流用してください。仕様書はありません』という。何か質問しても大抵『元のソースコード/ツールを見て下さい』と返される。
そのソースコード/ツールがちゃんと設計されて流用性も良いものなら良かったのだけれども、実際は元々ソフトウェア開発は専門外の人が片手間に作ったようなものらしく、設計もムチャクチャだしちょっと手を入れようものなら却ってバグが出てしまうような代物。
で、私がこのプロジェクトを引き継ぐ前の前任者は、そんな状況に加えて、開発中から仕様追加・変更等の要求がこちらのスケジュールを考えずに上がってくるのに辟易して、やっつけでとりあえず動くところまで作ったような状況でこのプロジェクトから離脱。
この時点でもう手を引いたほうが良かったのだけれども、追加開発を見込んで初期開発分を大安売りしちゃったせいで、営業的にはやめるにやめられない。
ちなみに肝心の追加開発は、こちらのスケジュールの都合がつかず (費用面の問題もあったか?) 結局一部 (というか大部分?) 他社に流れてしまったりしている。そもそも社内にソフトウェア開発者の手が不足しているのである。
で、私がその後を引き継いで (どう考えてもちゃんとした引き継ぎは行えていないけれど) 追加開発となったのですが、まずスタートが別プロジェクトの作業遅れに押されて遅れた上に、前任者のやっつけ仕事っぷりが想像以上で見積もり時の想定作業量を大幅にオーバー。追加要員も期待できず、残業と休日出勤でなんとか出来るところまでやろう、それでも納期に間に合うかどうかという状態。
一応私からは『このままじゃ無理!』というアラートは上げたのだけど、どうも対外的な立場もあってか営業から先へ上手く伝わらない感が。
このことから得られる教訓は、たくさんあると思うけどとりあえず…
参考:『おごちゃんの雑文』より『ダメな仕事を受けないためのNGワード』。『今回は予算があまりないので小額になりますが、次回はしっかり払いますので』に、まさに嵌ってしまったという感じ。
C プログラマ、特に組込系のプログラマにとって、移植性の高いコードを書く上で重要でありながら意外とハマる人が多いのが、タイトルに挙げた 3点だと思う。
エンディアンの変換で重要なのは、以下の 2点。
swap16/32 という関数/マクロ関数名をよく見かけるが、これだとどのエンディアンからどのエンディアンへ変更したいのかが分からないため、移植時に問題となる可能性がある。
アライメントに関しては、構造体のパディングは理解していても、例えば以下のようなコードで問題が発生する場合があることを理解していない人が多いように思う。
void fill_buffer(uint8_t *buf, uint32_t n)
{
*(uint32_t *)buf = n;
}
C 言語の仕様として、 buf が uint32_t 型の境界にアラインされていない場合の動作は未定義となる。例えば、 CPU によってはアドレスの下位 2ビットが無視されてしまい、誤ったアドレスに値を書き込んでしまう結果となることがある。
int 型のサイズに関しては、理解していてもなかなか難しい。私も以下のようなコードを書いてハマったことがある。
uint32_t n;
uint8_t buf[4];
/* … */
n = buf[0] * 0x1 + buf[1] * 0x100 + buf[2] * 0x10000 + buf[3] * 0x1000000;
演算子の優先順位、型変換の規則をきっちり理解して、コードを書く際には常に注意を払うことが必要だと思う。
そんなソースコードを渡されて、たいした説明も無く『参考にしてください』と言われても、まず解読に苦労してしまい、自分で開発するのとどっちが早いか分からないです。
あつくて寝られない時はhackしろ! 386BSD(98)はそうやってつくられましたよ? -- あるハッカー