質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

88.58%

C/C++でのコードの書き方について

解決済

回答 3

投稿

  • 評価
  • クリップ 0
  • VIEW 808

reasoku

score 12

1、C/C++でのコードの書き方について

最近C/C++の業務に移ったのですが、下記のようなメソッドを多く見かけます。

// ログ等処理1

while(1)
{
    if( IsValid1() == FALSE )
    {
        // エラーログ1
        break;
    }
    if( IsValid2() == FALSE )
    {
        // エラーログ2
        break;
    }
    if( IsValid3() == FALSE )
    {
        // エラーログ3
        break;
    }
    if( method4() == FALSE )
    {
        // エラーログ4
        break;
    }
    if( method5() == FALSE )
    {
        // エラーログ5
        break;
    }

    // やりたい処理6
    break;
}

// ログ等処理2


上記のような実装はC/C++では一般的な書き方なのでしょうか?

2、メソッドの分割について

また、処理が長いメソッドも多いのですが、メソッドを細かく分けると実行速度が遅くなる弊害が発生しやすいなどC/C++としてそういう傾向にあったりするのでしょうか?
細かく分けると遅くなる気はあまりしていないのですが…。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

+3

こんにちは。

上記のような実装はC/C++では一般的な書き方なのでしょうか?

C言語では比較的よく見かける記述のように感じます。昔のC言語は bool型がありませんでしたので少しでも意味を伝えようと FALSE と比較していました。
FALSEと比較すると2重否定になりがち(xx != FALSE)ですので TRUE と比較したくなりますが、C言語の定義上、!=0が真ですから間違ってTRUEと比較するとバグりますのでFALSEとの比較が推奨されていました。

しかし、その辺りのサポートが強力なC++ではあまり好ましくない書き方と思います。
C++には 最初から bool 型がありますから、素直に true と比較した方が分かりやすいです。
更に if文の条件式はbool型を期待しますから、わざわざif (xxx == true)などと書かず、if (xxx)と書いた方がスマートに見えます。
もし、各関数の戻り値が bool型でない場合は、戻り値をenum 型(scoped enumなら更に望ましい)で定義するとバグをコンパイラが検出しやすくなります。

また、処理が長いメソッドも多いのですが、メソッドを細かく分けると実行速度が遅くなる弊害が発生しやすいなどC/C++としてそういう傾向にあったりするのでしょうか?

細かいメソッドは、inline定義すると速度は落ちないです。
ただ、メソッドの粒度には要注意です。ロジックを整理できていない複雑なメソッドや、ロジックを整理できていたとしても細かすぎるメソッドは可読性を劣化させますので。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2018/11/05 16:23

    最近C++17の業務をやっているんですが、variantを使うといい感じになる(業務でC++17を使っているところがツチノコレベルで珍しい)。

    キャンセル

  • 2018/11/06 10:28

    >Chironianさん
    ご回答ありがとうございます。

    上の方の回答の方にも書きましたが、流用の本当に大本のソースがCで実装されていたようです。
    Cにはそんな歴史があったんですね。
    実際、関数の返却値を0を失敗1を成功としているものと0を正常1を異常としているものともあったりしてかなり混乱しました。
    C#をずっと書いてきたので、あまり失敗と成功をメソッドの返却値で返すイメージがなかったので。
    ちょうど単体試験のフェーズなので記載していただいたような修正は、出来る範囲で入れていこうかと思います。

    >EnumHackさん
    C++17ですか…。そこまで新しくすることは今の業務だとなさそうですね。。。VisualStudio2010で開発している環境ですので…。

    キャンセル

checkベストアンサー

+1

ソースが元々CだったかC上がりの人が書いたのでは?
結構古くからある方法でgoto有害論のおかげでgotoは悪だ!使用禁止!けどエラー時は特定の場所にまとめて飛びたい、というので生まれた手法だったかと。
使う使わないは人によると思いますが、パターンとしてそこまで珍しい書き方でもないかなー、という印象です。

C/C++としてそういう傾向にあったりするのでしょうか?
細かく分けると遅くなる気はあまりしていないのですが…。

言語としての傾向ではないと思います。
組み込みとのことなので「理由あって」関数を分けてないなら使用しているマイコンのためじゃないでしょうか?
・メモリが小さくてスタックがあまりとれないからあえて関数化をあまりしない
・割り込みとかタイムクリティカルな処理で関数を飛ぶ時間も惜しい
というのがぱっと思いつく理由です。

あとはまぁ、動いてる部分を変えたくないから少しずつ拡張を重ねたせいでダラダラ大きくなった、というのもよくある理由ですね。
大抵の場合はちゃんと関数化したほうがいいと思います。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2018/11/06 10:28

    ご回答ありがとうございます。

    流用の大本のソースがCで書かれていたようでした。goto回避で生まれたんですね。

    なぜそうなったか原因はわからないのですが、おそらく挙げて頂いたような理由ではなさそうなので、出来る範囲で読みやすく改善はしていきたいと思います。

    キャンセル

  • 2018/11/06 10:44 編集

    たぶんgoto回避のためのwhile-breakと混乱していると思います。
    質問文にご提示のソースはgoto回避構造にはなっていません。

    例えば、以下の方法ならgotoや例外、余分な関数を使わなくてもエラー処理をまとめる事ができます。
    int success=FALSE;
    do
    {
      if(エラー1)
    break;
      if(エラー2)
    break;

      success=TRUE;
      正常処理;
    }
    while(0);
    if (success == FALSE)
    {
      エラー処理
    }

    しかし、エラー処理を1箇所にまとめるための goto に反対する勢力はだんだんと少なくなっていった印象を受けています。却ってプログラムが読みにくくなり、本末転倒だったからと思います。

    キャンセル

  • 2018/11/06 23:50

    単にbreakで飛ぶ手法がgotoを使いたくないがために生まれて、(良いか悪いかはさておき)経験上こういった書き方もそこまで珍しいとは思わない、ということを言いたかっただけです。
    例示のオリジナルを書いた人は関数を抜ける際のログを残したいのでreturnの代わりとして使ったんじゃないですかね?
    組み込みなのでprintfデバッグの名残じゃないかと。

    キャンセル

+1

全体が見えないのでなんとも言えませんが、通常エラー処理は例外で捉えるようにすると思います。
また、すべての処理でbreakするならwhile()はいらないと思うのですが?

「追記」

2、メソッドの分割について

私は可能な限り小分けする方ですね。その方が見通しが良くなります。処理に結果は一つd^^
小分けして遅くなるような処理って、ドライバや組み込みでもなければ、今のCPUやコンパイラなら問題にならないと思います。

例えば、上の例だとIsValid()とmethod()で纏めて処理にして何が失敗したか復帰値で分かるようにするとか・・・

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2018/11/05 13:19

    回答ありがとうございます。

    プロジェクトとして基本的なエラーは例外で扱う仕組みにはしていないようで、本当に致命的なエラーだけ捕捉する作りになっています。

    while(1)はループ目的ではなく、どの箇所のbreak後にも同じログ処理のメソッドを呼びたいらしく、このような実装にしているようです。
    for(;;)で行っているものもありました。
    なぜこのような実装をしているのかわからなかったため、C/C++のお作法的な書き方としてそのようなものが存在するのかを確認したく、このような質問をした次第です。

    組み込み系なのですが、その場合はあまり細かくしないほうが良いのでしょうか?
    cateye様がおっしゃっている通り、処理に結果がひとつであるべきだと思っています。
    正直今プロジェクトに存在するコードが読みづらく、まず担当分のコードのメソッド分割からだけでも読みやすく直していければと思っておりまして…。

    キャンセル

  • 2018/11/05 14:04

    組み込み系であれば、割り込みやキュー制御などクリティカルな部分もあると思いますが、それ以外はあまり気にする事もないと思います。まず、ちゃんと動くコードを最適化は(くれぐれもデグレ起こさないように)その後で・・・私も最初は人が読めないようなコードを(いっぱいーー;)書いてましたが、ソースは読みやすいのが一番ですd^^

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 88.58%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る