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

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

ただいまの
回答率

88.62%

for/foreach文の中の一部だけが異なる場合のコードのきれいな書き方について。

解決済

回答 4

投稿 編集

  • 評価
  • クリップ 1
  • VIEW 3,457

LaLaLand

score 100

C#でコードを書いております。

似たコードを2か所書く必要があるのですが、それをよりエレガントな方法で書きたいと思っています。

似たコードは以下の2つ(検索と置換)です。 ともにGREP処理の中で必要になっているものです。

処理A ファイルの検索処理

foreach (var file in files)
{
   // 共通処理
  // ファイルをダウンロードして、
  // ユーザーが指定した文言で検索。

  // 個別処理
    // 検索の場合は何もしない。
}

処理B ファイルの置換処理 

foreach (var file in files)
{
  // 共通処理
  //  ファイルをダウンロードして、
  // ユーザーが指定した文言で置換。

   // 個別処理
  //  置換が終わったファイルをアップロード。
}

単純に考えれば、以下のようなやり方があるかなぁ? と思っています。 何かほかにやり方があれば、教えていただけると幸いです。 よろしくお願い致します。

public enum ShoriType
{
    // 検索
    Find,

    // 置換
    Replace,
}

public class MyClass
{
    public ShoriType ShoriType { get; set; }

    private readonly IDictionary<ShorType, Action> _actions;

    public MyClass()
    {
        this._actions = new ...;
        this._actions.Add(ShoriType.TypeA, ()=>{ /* 何もしない。*/});
        this._actions.Add(ShoriType.TypeB, this.Upload);
    }

  private void Upload(...)
    {
       // 置換後のアップロード処理。
    }

    public void Shori()
    {
        foreach (var x in xx)
        {
             // 共通処理
             ...

             // 個別処理
             this._actions[this.ShoriType]();
        }
    }
}
  • 気になる質問をクリップする

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

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

    クリップを取り消します

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

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

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

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

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

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

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

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

    質問の評価を下げる

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

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

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

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

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

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

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

    詳細な説明はこちら

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

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

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

質問への追記・修正、ベストアンサー選択の依頼

  • Tak1wa

    2015/12/03 23:31 編集

    その似た処理はどういう関係なのでしょうか。
    モデルの特性によって、継承か委譲か変わるのでは。処理Aと処理Bをどういうときに使い分けるのか例があると回答が出やすいかもしれません。

    キャンセル

  • LaLaLand

    2015/12/04 00:04 編集

    ありがとうございます。
    ----

    > その似た処理はどういう関係なのでしょうか。
    ----
    片方が検索。もう片方が置換処理です。
    サーバー上にあるファイルを正規表現などで検索または置換する処理を実装しています。
    検索または置換を行うロジック自体は上記の「共通処理」内で行っています。
    ----
    ----

    今回の質問は検索・置換が終わった後の処理についてです。
    フローは以下の通り。
    ----
    【検索】
    1. サーバーのファイルをダウンロードする。
    2. ダウンロードしたファイルを検索または置換する。
    ----

    【置換】
    1. サーバーのファイルをダウンロードする。
    2. ダウンロードしたファイルを検索または置換する。
    3. 置換が終わったファイルをアップロードする。

    ----
    > 処理Aと処理Bをどういうときに使い分けるのか
    ユーザーが処理A(検索)と処理B(置換)を呼び分けます。


    ====
    なお、2の処理は別クラスに委譲しております。
    そのため、検索なのか置換なのかを、この処理の中で意識したいとは思っていません。

    キャンセル

  • LaLaLand

    2015/12/04 00:12

    改行できない・・・(T T)

    キャンセル

回答 4

checkベストアンサー

+4

一つのループ内で異なる処理を選択的に実行するよりも、メソッドを分けて共通処理部分を共通化した方がいいように思いますが。

public void ShoriA()
{
    foreach(var x in xx)
    {
        // 共通処理
        kyotsuShori(x);
        // 個別処理
            :
    }
}

public void ShoriB()
{
    foreach(var x in xx)
    {
        // 共通処理
        kyotsuShori(x);
        // 個別処理
            :
    }
}

private void kyotsuShori(型 x)
{
    // 共通処理
}

どうしても一つの関数で別の仕事をさせたいのであれば、IDictionaryのような面倒なことをせずとも、「個別処理」を直接デリゲートで切り替えればコードはすっきりすると思います。

private Action<型> _kobetsuShori = kobetsuShoriA;

public void SetShoriType(ShoriType type)
{
    switch(type)
    {
    case ShoriType.TypeA:
        _kobetsuShori = kobetsuShoriA;
        break;
    case ShoriType.TypeB:
        _kobetsuShori = kobetsuShoriB;
        break;
    }
}

public void Shori()
{
    foreach(var x in xx)
    {
        // 共通処理
            :
        // 個別処理
        _kobetsuShori(x);
    }
}

private void kobetsuShoriA(型 x)
{
    // 処理A
}

private void kobetsuShoriB(型 x)
{
    // 処理B
}

少し質問内容が変わったようですね。個別処理Aが何もしないということであれば、一つのメソッドにまとめた方がすっきりするかもしれません。

public void Shori(bool replace)
{
    Action action = ()=>{};
    if(replace)
        action = this.Upload;

    foreach(var file in files)
    {
        // 共通処理
            :
        // 個別処理
        action();
    }
}

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/12/04 00:17

    回答ありがとうございます。
    この書き方もいいですよね。考えてみたいと思います。

    キャンセル

  • 2015/12/04 00:20

    少し質問内容が変わりましたね。回答の方に追加で書いてみました。

    キャンセル

  • 2015/12/04 10:49

    ありがとうございます。
    よりシンプルになってますね。

    キャンセル

+3

こんにちは。

私はユニットテスト作成スキルが低いこともあり、 出来るだけ外部から注入できるように作ります。

今回の例だと、複数のファイルを繰り返し処理をするディレクターに 処理方法とファイル一覧を渡してやってはどうでしょうかね。 でも実装量は増えている気もしますし、微妙かもしれませんね。

class Program
{
    static void Main(string[] args)
    {
        var director = new FileProcessDirector();
        var file = new FileInfo[10];

        //...

        director.Process(file, new SearchProcess());
    }
}

class FileProcessDirector
{
    public void Process(FileInfo[] files, IFileProcessable proc)
    {
        foreach(var file in files)
        {
            proc.Pre(file);

            proc.Process(file);

            proc.Post(file);
        }
    }
}

interface IFileProcessable
{
    void Pre(FileInfo file);
    void Process(FileInfo file);
    void Post(FileInfo file);
}

class SearchProcess : IFileProcessable
{
    public void Pre(FileInfo file)
    {
        //nothing
    }

    public void Process(FileInfo file)
    {
        //検索処理
    }

    public void Post(FileInfo file)
    {
        //nothing
    }

}

class ReplaceProcess : IFileProcessable
{
    public void Pre(FileInfo file)
    {
        //nothing
    }

    public void Process(FileInfo file)
    {
        //置換処理
    }
    public void Post(FileInfo file)
    {
        //アップロード
    }
}

#しまった、共通処理はどこでやれば良いだろう… ユーティリティクラスを用意してそれぞれのクラスから呼ぶようにするか、ファイル処理で必ず行うべき処理なのであればディレクタークラスで処理するもしくはファイル処理クラスのベースクラスに実装してやる感じでしょうかね。

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/12/04 08:47

    IFileProcessable <-- SearchProcess <-- ReplaceProcess
    という継承関係にして、
    ReplaceProcessではbase.Hoge();してやるのはどうでしょう

    キャンセル

  • 2015/12/04 08:56

    「何もしない検索」と「置換を伴う検索」と見れば、確かに汎化関係としても良さそうですね。共通処理は検索baseクラスに実装する感じですかね。

    キャンセル

  • 2015/12/04 10:41

    ありがとうございます。
    このやり方だと、「IFileProcessable」をインターフェースではなく抽象クラスにして
    その中に共通処理を書くか、共通処理を行うクラスをIFileProcessableに持たせてもいいでしょうね。

    キャンセル

+3

こんばんは。

C#から離れて久しいので直ぐには書けないのですが、下記の方法も考えられます。

  1. MyClassに仮想関数kobetsuShori()を設けます。
  2. MyClassを派生したMyClassAとMyClassBを作ります。
  3. そして、MyClassAとMyClassBの仮想関数kobetsuShori()に、それぞれkobetsuShoriA()とkobetsuShoriB()の内容を記述します。
  4. 最後に、MyClassのShori()の個別処理部は単純にkobetsuShori()を呼びます。

これにより、MyClassAのShori()を呼べばkobetsuShoriA()の内容で個別処理が実行されます。MyClassBのShori()を呼べばkobetsuShoriB()の内容で個別処理が実行されます。

kobetsuShoriA()、kobetsuShoriB()を呼び分けるためだけであれば、わざわざクラスを作るのもどうかと思いますが、そのような呼び分けをしたいケースでは、クラスを分けるとMyClassの他の部分もスマートに記述できる時も少なくないですよ。 MyClassの全体的な機能を見渡して、検討してみて下さい。

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/12/04 10:37

    ありがとうございます。
    派生クラスを使う方法ですか。
    少しの処理を切り分けるには大げさな方法かなぁ、と思ってしまいます。

    キャンセル

+1

enumがわかっているのであれば、拡張メソッドを使えばループ中に違いを意識しなくてよくなるので、読みやすくなると思います。

static class ShoriTypeExtension
{
  static IDictionary<ShorType, Action> _actions {
    {ShoriType.TypeA, () => {}},
    {ShoriType.TypeB, () => {Upload()}
  }

  public static Shori(this ShoriType type)
  {
    _actions[type]();
  }
}

public class Files
{
  public void Shori()
  {
    foreach(var file in files)
    {
        ...
        file.shoriType.SHori();
    }
  }
}

こんな感じです。 ファイル処理が長かったりShoriTypeが多い場合はクラスで分けるべきだと思いますが、処理が短いのであれば拡張メソッドもありだと思います。

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2015/12/04 10:35

    ありがとうございます。
    拡張メソッドを使う所がポイントですね。

    キャンセル

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

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

関連した質問

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