誰もこの代替案を提案していないことに驚いたので、質問はしばらく前からありましたが、追加します。この問題に対処する良い方法の 1 つは、変数を使用して現在の状態を追跡することです。gotoこれは、クリーンアップ コードに到達するために使用するかどうかに関係なく使用できる手法です。他のコーディング手法と同様に、長所と短所があり、すべての状況に適しているわけではありませんが、スタイルを選択する場合は検討する価値があります。特にgoto、深くネストされたifs にならないようにしたい場合は特にそうです。
基本的な考え方は、実行する必要がある可能性のあるすべてのクリーンアップ アクションに対して、クリーンアップを実行する必要があるかどうかを判断できる変数が存在するということです。
goto元の質問のコードに近いため、最初にバージョンを表示します。
int foo(int bar)
{
int return_value = 0;
int something_done = 0;
int stuff_inited = 0;
int stuff_prepared = 0;
/*
* Prepare
*/
if (do_something(bar)) {
something_done = 1;
} else {
goto cleanup;
}
if (init_stuff(bar)) {
stuff_inited = 1;
} else {
goto cleanup;
}
if (prepare_stuff(bar)) {
stufF_prepared = 1;
} else {
goto cleanup;
}
/*
* Do the thing
*/
return_value = do_the_thing(bar);
/*
* Clean up
*/
cleanup:
if (stuff_prepared) {
unprepare_stuff();
}
if (stuff_inited) {
uninit_stuff();
}
if (something_done) {
undo_something();
}
return return_value;
}
他のいくつかの手法に対するこれの利点の1つは、初期化関数の順序が変更された場合でも、正しいクリーンアップが行われることです。たとえば、switch別の回答で説明されている方法を使用して、初期化の順序が変更された場合、switch最初に実際に初期化されていないものをクリーンアップしようとするのを避けるために、非常に慎重に編集する必要があります。
さて、この方法は余分な変数を大量に追加すると主張する人もいるかもしれませんが、実際には、既存の変数が必要な状態をすでに追跡しているか、追跡するように作成できることがよくあります。たとえば、が実際に、またはprepare_stuff()の呼び出しである場合、返されたポインタまたはファイル記述子を保持する変数を使用できます。たとえば、次のようになります。malloc()open()
int fd = -1;
....
fd = open(...);
if (fd == -1) {
goto cleanup;
}
...
cleanup:
if (fd != -1) {
close(fd);
}
ここで、さらに変数を使用してエラー ステータスを追跡すると、goto初期化が必要になるほどインデントが深くなることなく、完全に回避でき、正しくクリーンアップできます。
int foo(int bar)
{
int return_value = 0;
int something_done = 0;
int stuff_inited = 0;
int stuff_prepared = 0;
int oksofar = 1;
/*
* Prepare
*/
if (oksofar) { /* NB This "if" statement is optional (it always executes) but included for consistency */
if (do_something(bar)) {
something_done = 1;
} else {
oksofar = 0;
}
}
if (oksofar) {
if (init_stuff(bar)) {
stuff_inited = 1;
} else {
oksofar = 0;
}
}
if (oksofar) {
if (prepare_stuff(bar)) {
stuff_prepared = 1;
} else {
oksofar = 0;
}
}
/*
* Do the thing
*/
if (oksofar) {
return_value = do_the_thing(bar);
}
/*
* Clean up
*/
if (stuff_prepared) {
unprepare_stuff();
}
if (stuff_inited) {
uninit_stuff();
}
if (something_done) {
undo_something();
}
return return_value;
}
繰り返しますが、これには潜在的な批判があります。
- これらすべての「if」がパフォーマンスを損なうことはありませんか? いいえ - 成功した場合は、とにかくすべてのチェックを行う必要があるためです (そうしないと、すべてのエラー ケースをチェックするわけではありません)。失敗した場合、ほとんどのコンパイラは失敗した
if (oksofar)チェックのシーケンスをクリーンアップコードへの単一のジャンプに最適化します (GCC は確かにそうします) - いずれにせよ、エラーの場合は通常、パフォーマンスにとってそれほど重要ではありません。
これはさらに別の変数を追加していませんか? この場合はそうですが、多くの場合、ここreturn_valueで果たしている役割を果たすために変数を使用できoksofarます。一貫した方法でエラーを返すように関数を構成するとif、それぞれのケースで 2 番目のエラーを回避することもできます。
int return_value = 0;
if (!return_value) {
return_value = do_something(bar);
}
if (!return_value) {
return_value = init_stuff(bar);
}
if (!return_value) {
return_value = prepare_stuff(bar);
}
このようなコーディングの利点の 1 つは、一貫性があるということは、元のプログラマーが戻り値をチェックするのを忘れていた場所が痛々しい親指のように突き出ていることを意味し、(その 1 つのクラスの) バグを見つけるのがはるかに簡単になることです。
つまり、これは (まだ) この問題を解決するために使用できるもう 1 つのスタイルです。正しく使用すると、非常にクリーンで一貫性のあるコードが可能になります。他の手法と同様に、悪意のある人の手に渡れば、長くて混乱を招くコードが生成される可能性があります:-)