解決済み

returnと条件式内の代入

  • 暇なときにでも
  • 質問No.4170303
  • 閲覧数717
  • ありがとう数4
  • 気になる数0
  • 回答数4
  • コメント数0

お礼率 100% (4/4)

Cでmemcmpライクな関数が必要になり作りました。(正確には中でmemcmpを何回か呼び出して比較をする関数)
そのときに出た疑問なのですが、ループ内でif文→returnとするのと、ループの条件にif文の条件を織り込んで、関数の最後でreturnするのとどちらが良いのか。
作った後で、いくつかのサイトでreturnはまとめるほうがいいとありましたので2つめの関数も考えました。しかし、条件式に=(代入)を書くのは好ましくないとする考えもあるようで、もう一つ作りました。しかし、初期化直後のretを比較するのは気分が悪いのです。悩んでいます。つまり、次のような関数でより好ましいのはどれでしょうか? (いずれも質問用にmemcmpに書き直してます)

int mymemcmp(const void *a, const void *b, size_t cnt)
{
  const unsigned char *ap = a, *bp = b;
  int i, ret;
  for (i=0; i<cnt; i++)
  {
    ret = ap[i] - bp[i];
    if (ret != 0)
    {
      return ret;
    }
  }
  return 0;
}

int mymemcmp(const void *a, const void *b, size_t cnt)
{
  const unsigned char *ap = a, *bp = b;
  int i, ret=0;
  for (i=0; i<cnt && (ret = ap[i] - bp[i]) == 0; i++)
    ;
  return ret;
}

int mymemcmp(const void *a, const void *b, size_t cnt)
{
  const unsigned char *ap = a, *bp = b;
  int i, ret=0;
  for (i=0; ret == 0 && i<cnt; i++)
  {
    ret = ap[i] - bp[i];
  }
  return ret;
}

質問者が選んだベストアンサー

  • 回答No.4

ベストアンサー率 52% (391/751)

個人的には1を推奨します。
#2のかたが、述べているように、ソースをみたときのわかりやすさを優先すべきです。その意味では1と3が候補になります。
それで、1と3のどちらが良いかというと、この状態では、どちらでも良いでしょう。しかしながら、もし、forのネストが3重とかになったときに、一つの命令でループを抜ける方法は、C言語の場合、return文以外にありません。
その意味で、今後のことを考えると、1を推奨します。
今回の関数の要件は、「値が異なることが判明した時点で、処理終了となり、かつ、値が異なることを結果として返す。」ことが求められています。
もし、for文が3重になるような、ループで、上記の要件を満足する場合、1の方法が、良いと考えます。
以下、一般論になりますが、もし、処理中に異常が判明し、それ以上処理をする必要がないなら、即リターンする形式の方が、ソースがみやすくなります。
どちらのソースが簡潔か比較して下さい
-------------------------
以下ソース1
Aの判定
if Aのエラー発生 return;
Bの判定
if Bのエラー発生 return;
Cの判定
if Cのエラー発生 return;
全て正常な場合の処理

-------------------------
以下ソース2
Aの判定
if Aが正常{
 Bの判定
 if Bが正常{
   Cの判定
   if Cが正常{
     全て正常な場合の処理
   }
 }
}
お礼コメント
S117

お礼率 100% (4/4)

たしかに、異常系はその場でreturnするのがシンプルで良いですよね。
ただ、今回は異常系とも微妙に違うので悩みどころです。

回答ありがとうございました。
投稿日時 - 2008-07-12 11:43:35
Be MORE 7・12 OK-チップでイイコトはじまる

その他の回答 (全3件)

  • 回答No.3

ベストアンサー率 58% (14/24)

>returnはまとめるほうがいい
>条件式に=(代入)を書くのは好ましくない

確かにその通りなので以下提案です。
1番目の以下の部分をこう変えたら、上2つの条件が満足すると思います。
int mymemcmp(const void *a, const void *b, size_t cnt)
{
  const unsigned char *ap = a, *bp = b;
  int i, ret;
  for (i=0; i<cnt; i++)
  {
    ret = ap[i] - bp[i];
    if (ret != 0)
    {
// 変更箇所 return ret;
break;
    }
  }
// 変更箇所   return 0;
  return ret;
}

後、質問点とはまったく関係ない提案なのですが、キャストがcharだと長文の場合処理速度が遅くなるんでint型も利用して速度向上をめざす。
という具合でどうでしょう。
お礼コメント
S117

お礼率 100% (4/4)

回答ありがとうございます。なぜか考えているときにbreakを忘れてたんですよね。混乱してたのがだいぶ落ち着きました。

本来の(この疑問が生まれた)関数では、さらにライブラリのmemcmpを呼び出しています。しかし、このmymemcmpを書いたときにはint型なんて忘れてました。ありがとうございました。
投稿日時 - 2008-07-12 11:35:02
  • 回答No.2

ベストアンサー率 47% (287/608)

なにをもって「良い」と判断するのかで答えは変わると思います。
私の場合は処理速度にそれほど関係しない部分であれば
後から見て何をやっているのかがわかりやすいことを持って「良い」プログラムと考えます。
ですので、3番目あたりを良いプログラムであると判断します。
1番目はループの終了条件のi<cntとret==0の関係が分かりにくい。
2番目はループ内で何をしているかがわかりにくい。
3番目であればループ内で何をしているか終了条件が何であるかが分かりやすいです。

処理速度を考えれば2番目が最速かと思われますが、そのあたりはコンパイラの最適化が
どうとでもしてくれる範囲であると思いますし、実際の処理時間もミリ秒の差もでないでしょう。
お礼コメント
S117

お礼率 100% (4/4)

回答ありがとうございます。ソースの見易さについての質問でした。質問文に書き忘れていました。
投稿日時 - 2008-07-12 11:06:58
  • 回答No.1

ベストアンサー率 37% (122/322)

好みは人それぞれでしょうが、僕ならたぶんこう書きます。
  for (i=0; i<cnt; i++) {
    ret = ap[i] - bp[i];
    if (ret != 0) {
      break;
    }
  }
  return ret;
forの中を複雑にしたくないのと、出力は最後にまとめたいからです。
お礼コメント
S117

お礼率 100% (4/4)

回答ありがとうございます。なるほど、単にbreakでもいいですね。
投稿日時 - 2008-07-12 11:08:12
AIエージェント「あい」

こんにちは。AIエージェントの「あい」です。
あなたの悩みに、OKWAVE 3,500万件のQ&Aを分析して最適な回答をご提案します。

こんな書き方もあるよ!この情報は知ってる?あなたの知識を教えて!
このQ&Aにはまだコメントがありません。
あなたの思ったこと、知っていることをここにコメントしてみましょう。

その他の関連するQ&A、テーマをキーワードで探す

キーワードでQ&A、テーマを検索する

特集


より良い社会へ。感謝経済プロジェクト始動

ピックアップ

ページ先頭へ