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

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

新規登録して質問してみよう
ただいま回答率
85.50%
C#

C#はマルチパラダイムプログラミング言語の1つで、命令形・宣言型・関数型・ジェネリック型・コンポーネント指向・オブジェクティブ指向のプログラミング開発すべてに対応しています。

Q&A

解決済

4回答

5931閲覧

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

LaLaLand

総合スコア107

C#

C#はマルチパラダイムプログラミング言語の1つで、命令形・宣言型・関数型・ジェネリック型・コンポーネント指向・オブジェクティブ指向のプログラミング開発すべてに対応しています。

0グッド

1クリップ

投稿2015/12/03 14:11

編集2015/12/03 14:58

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

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

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

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

c#

1foreach (var file in files) 2{ 3 // 共通処理 4 // ファイルをダウンロードして、 5 // ユーザーが指定した文言で検索。 6 7 // 個別処理 8 // 検索の場合は何もしない。 9} 10

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

c#

1foreach (var file in files) 2{ 3 // 共通処理 4 // ファイルをダウンロードして、 5 // ユーザーが指定した文言で置換。 6 7 // 個別処理 8 // 置換が終わったファイルをアップロード。 9} 10

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

c#

1 2public enum ShoriType 3{ 4 // 検索 5 Find, 6 7 // 置換 8 Replace, 9} 10 11public class MyClass 12{ 13 public ShoriType ShoriType { get; set; } 14 15 private readonly IDictionary<ShorType, Action> _actions; 16 17 public MyClass() 18 { 19 this._actions = new ...; 20 this._actions.Add(ShoriType.TypeA, ()=>{ /* 何もしない。*/}); 21 this._actions.Add(ShoriType.TypeB, this.Upload); 22 } 23 24 private void Upload(...) 25 { 26 // 置換後のアップロード処理。 27 } 28 29 public void Shori() 30 { 31 foreach (var x in xx) 32 { 33 // 共通処理 34 ... 35 36 // 個別処理 37 this._actions[this.ShoriType](); 38 } 39 } 40}

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

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

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

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

Tak1wa

2015/12/03 14:32 編集

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

2015/12/03 15:08 編集

ありがとうございます。 ---- > その似た処理はどういう関係なのでしょうか。 ---- 片方が検索。もう片方が置換処理です。 サーバー上にあるファイルを正規表現などで検索または置換する処理を実装しています。 検索または置換を行うロジック自体は上記の「共通処理」内で行っています。 ---- ---- 今回の質問は検索・置換が終わった後の処理についてです。 フローは以下の通り。 ---- 【検索】 1. サーバーのファイルをダウンロードする。 2. ダウンロードしたファイルを検索または置換する。 ---- 【置換】 1. サーバーのファイルをダウンロードする。 2. ダウンロードしたファイルを検索または置換する。 3. 置換が終わったファイルをアップロードする。 ---- > 処理Aと処理Bをどういうときに使い分けるのか ユーザーが処理A(検索)と処理B(置換)を呼び分けます。 ==== なお、2の処理は別クラスに委譲しております。 そのため、検索なのか置換なのかを、この処理の中で意識したいとは思っていません。
LaLaLand

2015/12/03 15:12

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

回答4

0

ベストアンサー

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

C#

1public void ShoriA() 2{ 3 foreach(var x in xx) 4 { 5 // 共通処理 6 kyotsuShori(x); 7 // 個別処理 8 : 9 } 10} 11 12public void ShoriB() 13{ 14 foreach(var x in xx) 15 { 16 // 共通処理 17 kyotsuShori(x); 18 // 個別処理 19 : 20 } 21} 22 23private void kyotsuShori(型 x) 24{ 25 // 共通処理 26}

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

C#

1private Action<> _kobetsuShori = kobetsuShoriA; 2 3public void SetShoriType(ShoriType type) 4{ 5 switch(type) 6 { 7 case ShoriType.TypeA: 8 _kobetsuShori = kobetsuShoriA; 9 break; 10 case ShoriType.TypeB: 11 _kobetsuShori = kobetsuShoriB; 12 break; 13 } 14} 15 16public void Shori() 17{ 18 foreach(var x in xx) 19 { 20 // 共通処理 21 : 22 // 個別処理 23 _kobetsuShori(x); 24 } 25} 26 27private void kobetsuShoriA(型 x) 28{ 29 // 処理A 30} 31 32private void kobetsuShoriB(型 x) 33{ 34 // 処理B 35}

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

C#

1public void Shori(bool replace) 2{ 3 Action action = ()=>{}; 4 if(replace) 5 action = this.Upload; 6 7 foreach(var file in files) 8 { 9 // 共通処理 10 : 11 // 個別処理 12 action(); 13 } 14}

投稿2015/12/03 15:04

編集2015/12/03 15:19
catsforepaw

総合スコア5938

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

LaLaLand

2015/12/03 15:17

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

2015/12/03 15:20

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

2015/12/04 01:49

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

0

こんばんは。

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/03 15:36

編集2015/12/03 15:58
Chironian

総合スコア23272

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

LaLaLand

2015/12/04 01:37

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

0

こんにちは。

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

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

C#

1class Program 2{ 3 static void Main(string[] args) 4 { 5 var director = new FileProcessDirector(); 6 var file = new FileInfo[10]; 7 8 //... 9 10 director.Process(file, new SearchProcess()); 11 } 12} 13 14class FileProcessDirector 15{ 16 public void Process(FileInfo[] files, IFileProcessable proc) 17 { 18 foreach(var file in files) 19 { 20 proc.Pre(file); 21 22 proc.Process(file); 23 24 proc.Post(file); 25 } 26 } 27} 28 29interface IFileProcessable 30{ 31 void Pre(FileInfo file); 32 void Process(FileInfo file); 33 void Post(FileInfo file); 34} 35 36class SearchProcess : IFileProcessable 37{ 38 public void Pre(FileInfo file) 39 { 40 //nothing 41 } 42 43 public void Process(FileInfo file) 44 { 45 //検索処理 46 } 47 48 public void Post(FileInfo file) 49 { 50 //nothing 51 } 52 53} 54 55class ReplaceProcess : IFileProcessable 56{ 57 public void Pre(FileInfo file) 58 { 59 //nothing 60 } 61 62 public void Process(FileInfo file) 63 { 64 //置換処理 65 } 66 public void Post(FileInfo file) 67 { 68 //アップロード 69 } 70}

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

投稿2015/12/03 15:33

編集2015/12/03 15:38
Tak1wa

総合スコア4791

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

ozwk

2015/12/03 23:47

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

2015/12/03 23:56

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

2015/12/04 01:41

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

0

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

c#

1static class ShoriTypeExtension 2{ 3 static IDictionary<ShorType, Action> _actions { 4 {ShoriType.TypeA, () => {}}, 5 {ShoriType.TypeB, () => {Upload()} 6 } 7 8 public static Shori(this ShoriType type) 9 { 10 _actions[type](); 11 } 12} 13 14public class Files 15{ 16 public void Shori() 17 { 18 foreach(var file in files) 19 { 20 ... 21 file.shoriType.SHori(); 22 } 23 } 24}

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

投稿2015/12/03 17:34

mrasu

総合スコア21

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

LaLaLand

2015/12/04 01:35

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

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

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

ただいまの回答率
85.50%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問