LoginSignup
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@shun310

今年学んだ良いコーディングスタイル【処理・関数編】

Last updated at Posted at 2024-12-31

はじめに

大学研究室（シミュレーション系）からIT業界に就職した新卒エンジニアが「これ面白いな～」「これ役立つな～」と思ったコーディングスタイルについて書いていきます。
コーディングスタイルに関する良書である『リーダブルコード』『Good Code, Bad Code』の内容をベースに、自分の経験や先輩エンジニアの言葉を入れて再編しています。

ここでは、処理周り（コードの見た目や論理を整える方法）に加え、関数に関するノウハウについて述べていきます。コード例は基本的にC#で書かれています。

更新履歴

2025/1/13
コメントにてご教授いただいた点を更新しました。

処理編

インデントや段落を整える

コードはその見た目が整っている方が見やすいです。当たり前～
if文やfor文の中身をインデントするのは当然として、

当たり前.cs
for (int i = 0; i < 10; i++)
{
    if (i == 0)
    {
        Console.WriteLine("first call");
    }
    else
    {
        Console.WriteLine(i);
    }
}

以下のようなケースでは引数の先頭を揃えることも意識すべきでしょう。

関数の呼び出し.cs
//                     |ここを揃える
string.Concat("abc",    "123");
string.Concat("aaaaa",  "bbbbb");
string.Concat("hoge",   "huga");

(2023/1/13追記)
↑の引数揃えですが、Gitの影響範囲が大きくなってしまうため、あまり推奨されないケースが多いようです。コードアナライザや自動フォーマッタのルールとの衝突も懸念されますね...。
上記のような自動化ツールを使うのが修正の手間やコーディングの一貫性の観点からは良いと考えます。

ネストを浅くする

ネストが深いと可読性が下がります。
その中で変数が不意に変更されようものなら大混乱です。

例えば、以下のような場合にネストは深くなる傾向にあります。
関数編に絡んでくるため、ここでは紹介だけ。

ループの見やすさは for >= while >> do-while

諸説あるかもですが、for文は「初期値・終了条件・更新を明示的に書ける」点で優れます。
逆に、do-while文は「ループ内が何回実行されるか直感的に分かりづらい」「終了条件がループ処理より下にあり、流れが読みづらい」という欠点があります。
以上の理由から、個人的にはループはfor文を基本とし、whileを使うとスマートに書けるときにはwhileを使うようにしています。

条件式は「気になる」条件を先に書く

条件式の書き方には下記のような原則があります。

  • 否定形より肯定形
  • 左辺に変数（比較されるもの）、右辺に定数（比較するもの）を書く
  • 異常系より正常系を先に書く。ただし、異常系であってガード句に当たる条件は正常系より先に書く
  • 単純な条件をifに書き、そうでない場合をelseでキャッチする

ただ、他人がコードを読むことを考えると、まず「気になる」条件を先に書くことが大原則と言えるでしょう。

例えば、下記のようなファイルパスチェックは良く行われると思います。
このif文には否定形(!)を含みますが、ファイルを開く前に気にすべき条件なのでこの書き方で十分読みやすいですね。

気になる条件.cs
if (!File.Exists(path))
{
    // デフォルトのファイルを作成する処理など
}
else
{
    using (FileStream fs = File.Open(path, FileMode.Open))
    {
        // ファイルを読む処理
    }
}

この論理を逆にすると、「ファイルパスが存在しないときはどうするんだ...？🤔」と考えながらコードを読むことになります（好みにもよると思いますが...）。

気になる....cs
if (File.Exists(path))
{
    using (FileStream fs = File.Open(path, FileMode.Open))
    {
        // ファイルを読む処理
    }
}
else
{
    // デフォルトのファイルを作成する処理など
}

関数編

下位問題を解く処理を関数にする

『リーダブルコード』にはエンジニアリングの核心についての下記の記述があります。

　エンジニアリングとは、大きな問題を小さな問題に分割して、それぞれの解決策を組み立てることに他ならない。この原則をコードに当てはめれば、堅ろうで読みやすいコードになる。

ということで、「最終的に解きたい問題」は「複数の独立した下位問題」に分割して解きましょう。この下位問題を解くための処理を関数にまとめます。
分解して関数にすれば、読みやすいだけでなく、再利用も容易になります。

下位問題を解く.cs
// 以前書いた記事の引用です
// https://qiita.com/shun310/items/6147fe529aa8fceca32d
using SomeLibrary;

public static void Main(string[] args)
{
    bool hasSentMessage = DeliverMessageToServer(url: "some URL", message: "Hello!");
}

/// <summary>
/// サーバにメッセージを送信する。
/// </summary>
/// <param name="url">サーバのURL</param>
/// <param name="message">メッセージ</param>
private static bool DeliverMessageToServer(string url, string message)
{
    try
    {
        // この関数の主目的を3つの下位問題に分けている
        SomeLibrary.Connect(url);
        SomeLibrary.Send(message);
        SomeLibrary.Close();
    }
    catch (ex 何かしらエラー)
    {
        return false;
    }
    return true;
}

特に、「上位問題とは（直接的に）無関係な下位問題を解く関数」はいわゆるユーティリティとして、プロジェクトをまたいで再利用できます。積極的に分解しましょう。

関数から早く戻る

ここでearly return（またはガード句）のお時間です。
要は、「ある条件を満たしたらreturn文まで何もしない」なら、「ある条件を満たした時点で関数からreturnしよう」ということです。1つの関数内にreturn文はいくつあっても良いのです。

EarlyReturn.cs
// 素数かどうか判定する
public bool IsPrime(int number)
{
    if (number <= 0)
    {
        return false;
    }

    if (number <= 2)
    {
        return true;
    }

    if (number % 2 == 0)
    {
        return false;
    }
    ...
}

フラグ変数を使っても似たようなことはできますが、「フラグ変数の値が何なのか」を気にする必要がありますし、実装の仕方がまずいと下記のようにネストがえらいこっちゃになります。

えらいこっちゃ.cs
// 素数かどうか判定する
public bool IsPrime(int number)
{
    bool isPrime = false;
    if (number <= 0)
    {
        isPrime = false;
    }
    else
    {
        if (number <= 2)
        {
            isPrime = true;
        }
        else
        {
            if (number % 2 == 0)
            {
                isPrime = false;
            }
            else
            {
                ...
            }
        }
    }
    return isPrime;
}

想定外の副作用に注意する

コードの副作用とは、「関数がその戻り値以外によって何かしら影響を与えること」を指す言葉です。下記は具体例です。

  • 関数外の変数の変更
  • ユーザへの出力表示
  • ファイル出力

意図的に副作用を起こす分には問題ないのですが、「想定外の副作用」は悪さをする可能性があります。例えば、

  • ある関数を呼んだとき、知らず知らずのうちにグローバル変数が書き換わっていた
  • 関数を呼ぶ度にユーザへの出力表示を走らせるので、想定外の時間がかかる
  • 関数を呼ぶ度にファイルを新しく作るので、記憶装置の容量を余計に食う

もし関数に副作用を持たせるなら、それに即した名前を付けましょう。「俺、副作用起こします！」ということが関数名からにじみ出るように。

俺、副作用起こします！.cs
public void CreateLogFile()
{
    // ログファイルを作成する処理
}

実引数の変更に注意する

こちらも副作用の一種です。関数の引数として渡したインスタンスや配列（あるいは、refキーワードなどにより「値の参照渡し」された変数）は変更できてしまいます。
こちらも意図的に変更する分には問題ないのですが、「うっかり」変えてしまうと関数の呼び出し元で悪さする可能性が高いです。

うっかり.cs
static void Main(string[] args)
{
    int[] indexes = { 0, 1, 2 };
    DisplayFirstIndex(indexes);
    Console.WriteLine(indexes[0]);  // 1が出力される（！）
}

/// <summary>
/// インデックス配列の最初の値を（1始まりに直して）出力する。
/// </summary>
/// <param name="indexes">インデックス配列</param>
public static void DisplayFirstIndex(int[] indexes)
{
    // ただ0始まりから1始まりにしたかっただけなのに
    Console.WriteLine(++indexes[0]);  // 1が出力される
}

引数を変更する意図がない（出力パラメータとして使わない）ならば、その引数を操作する前にコピーしてもよいでしょう。
（2025/1/13追記）
そもそもの要素がimmutableならば、このような対策は不要です。
また、C#ならばIEnumerable<T>という単純な列挙用（≒読み取り専用）のインターフェースが用意されているため、引数をこの型で渡すのも手だそうです。

安全にコピー.cs
static void Main(string[] args)
{
    int[] indexes = { 0, 1, 2 };
    DisplayFirstIndex(indexes);
    Console.WriteLine(indexes[0]);  // 0が出力される
}

/// <summary>
/// インデックス配列の最初の値を（1始まりに直して）出力する。
/// </summary>
/// <param name="indexes">インデックス配列</param>
public static void DisplayFirstIndex(int[] indexes)
{
    int[] _indexes = (int[])indexes.Clone();  // これで安心
    Console.WriteLine(++_indexes[0]);  // 1が出力される
}

戻り値のデフォルトを検討する

「関数内で設定値が取得できないとき、デフォルト値を戻す」という処理は良く行われます。
デフォルト値の候補としては以下のようなものが挙げられるでしょう。

  • 「狭義の」（？）デフォルト値（戻り値がintなら0, -1など）
  • 戻り値の型における空の値（空配列, 空文字列など。Null Objectパターンもこれに当たる）
  • null

どれをデフォルト値にすればよいかはケースバイケースです。
ただし、最近（というかほぼ全て？）の言語にはnull安全やオプショナルがあるため、関数からはちゃんとnullを返す方が好まれるようです。

『Good Code, Bad Code』には下記のような議論があります。参考まで。

  • 戻り値もマジックナンバー禁止！
  • 下位レイヤーでは「狭義の」デフォルト値を返すよりnullを返した方が良い（上位レイヤーでnullを受け取ったそのときにデフォルト値を設定すべき）
  • 空の値を返すことによってnullチェックを回避できるが、これは時に危険（上位レイヤーでは、あたかも空の値が「存在する」かのようにふるまう可能性がある。失敗時処理がかえって難しくなる）
  • nullを返すと上位レイヤーでnullチェックが必要だが、裏を返せば「上位レイヤーで適切な処理をする機会が与えられる」ということでもある

サードパーティAPIをラップする関数を作る

サードパーティAPI、というか、自分では実装の詳細を触れないAPIはラッパー関数で隠蔽するといいことが多いです。例えば、

  • APIを使う上で自分にとって必要な処理をラッパー関数によって補える
  • APIの仕様の変更をラッパー関数によって吸収できる
    など。
ラッパー関数.cs
public void SomeApiWrapper(...)
{
    // 何か前処理
    SomeApi(...);
    // 何か後処理
}

おわりに

【クラス・ユニットテスト編】を考え中です。いつか書きます。

参考文献

  • Dustin Boswell (著), Trevor Foucher (著), 角 征典 (翻訳)、『リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice) 』、2012
  • TomLong (著), 秋勇紀 (翻訳), 高田新山 (翻訳), 山本大祐 (監訳) 、『Good Code, Bad Code ～持続可能な開発のためのソフトウェアエンジニア的思考』、2023
1
0
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?