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

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

質問する

ただいまの
回答率

88.37%

コードを柔軟にしたい

受付中

回答 2

投稿 ・編集

  • 評価
  • クリップ 5
  • VIEW 1,504
syogakusya

syogakusya

score 67

質問

ある要件を満たすコードをより柔軟にしたいです。
要件は以下です。

あるデータベースのテーブルAのデータをBに移し、Bを空にしたい。このとき、以下の順に処理を行う。
1 トランザクションを開始
2   Bのレコードをすべて削除
3   Aからデータを抽出し、Bに移す(データは似通っていて、データの変換は単純)
4   Aのレコードをすべて削除
5   トランザクションを終了

1'途中で例外的状況に陥ったらロールバックしてログを出力して異常終了

以下、捕捉となります。

データベース操作はEntity Frameworkによって行う

このとき、以下の実装を行いました。
設定値などはところどころ省略しています。注目していただきたいのはDataAToBクラスです。 

class DataAToB
{
    private readonly ISetting setting;

    private readonly IMapper<A, B> mapper;

    public DataAToB(ISetting setting, IMapper<A, B> mapper)
    {
        this.setting = setting;
        this.mapper = mapper;
    }

    // データを移す処理はこれ
    public void Do()
    {
        // 1
        using (var context = new EntityFrameworkOrdinaryContext(setting))
        using (var subContext = new EntityFrameworkOrdinaryContext(setting))
        using (var transaction = context.Database.BeginTransaction())
        {
            try
            {
                // 2
                context.Bs.Delete();

                // 3
                foreach (var dataOfA in subContext.As)
                {
                    // 捕捉:マッピングは絶対成功する
                    var dataForB = this.mapper.Map(dataOfA);
                    context.Bs.Add(dataForB);
                }

                // 4
                context.As.Delete();

                // 5
                context.SaveChanges();
                transaction.Commit();
            }
            catch (Exception)
            {
                // 1'
                transaction.Rollback();
                throw new AToBFailureException();
            }
        }
    }
}

class EntityFrameworkOrdinaryContext : DbContext
{
    public EntityFrameworkOrdinaryContext(ISetting setting) : base(null)
    {
    }

    public DbSet<A> As { get; }

    public DbSet<B> Bs { get; }
}

interface ISetting
{
}

interface IMapper<TRaw, TMapped>
{
    TMapped Map(TRaw raw);
}

class A
{
}

class B
{
}

class AToBFailureException : Exception
{
     //...
}

しかし、このコードに改善の余地があるような気がしています。
以下が気になっているところです。

1.コンテキストクラスを直接生成している
2.Ｂからデータを削除し、BからAにデータを移し、Aのデータを削除するという具体的な一連の処理を直接やっている。
3.さらにトランザクション管理までしている。
4.データベースコンテキストクラスや、EntityFrameworkに直接依存している(DbContext,DbSetなど)

AからBでなくBからA、もっと言えばあらゆるエンティティからエンティティに対して、さらに言えば機能を分割してトランザクションがないパターンにも対応でき、あらゆるデータのストレージ間に対して汎用的に適用できるようなインターフェースと実装を書けたりしないものかと思い質問しました。必要に駆られているので教えてほしいといった旨ではございません。目的はコードをよくすることと、良いコードをかけるようになるための経験を積むことです。このコードは改善可能か、どういった観点から改善の方針を定めるのか、逆にこのままで十分なのかなど、皆様の意見をなんでも頂ければと思います。
よろしくお願いいたします。

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

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

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

    クリップを取り消します

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

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

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

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

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

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

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

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

    質問の評価を下げる

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

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

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

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

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

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

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

    詳細な説明はこちら

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

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

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

回答 2 件

link

0

あらゆるデータのストレージ間に対して汎用的に適用できるようなインターフェースと実装を書けたりしないものか

それってある程度System.Dataにまとまってませんか
お話に出ているEntityFramework自体、ある程度特化した環境で生産性をあげようと動くもの(作られてきたもの)なので、それを汎化したいといってもムリがあるのでは無いでしょうか？

最近teratailで見かける気がする「コードをよくすること」とはなんぞやというのが個人的な感想です
これこそ完全にコンテキストに依存するので回答しにくいような気がします
例えば私なら「データのストレージ間に対して汎用的に適用できるような...」の課題にそもそも「EntityFramework」は採用しませんし

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

link

0

こんにちは。
いい感じに抽象化の闇に飲まれてますね。昔自分も同じようにハマったりしました。質問がふわっとしているため、深読みしつつ回答してみます。

コードを柔軟にする、即ち汎用化、一般化していくことは、基本的には良いことと言われるものですが、実際に着手する際にはまず「どこまで一般化するのか？」の基準をはっきりと定める必要があります。
例えば、差し替えられる箇所を増やすほどに、そのコードを応用できる場所も増えるわけですが、それに伴って「（そのクラスを使用する）ユーザが書かなければならないコード」の量も増えていきます。そして、究極の汎用化は「何も書かない」ことになります。ただのC#まで行ってしまえば、ユーザはなんでも作れるわけですからね。
質問でやりたい目的は、「EntityFramework」そのものが既に達成しているため、この上に抽象レイヤをかぶせるのは蛇足であると考えます。質問のコードの時点で「やりたいことだけを書く」という状態までたどり着いていると判断できるからです。

ただし、特定の用途に対して繰り返し使いたい汎用的な処理を作成するという方針自体は悪くないです。これは「EntityFrameworkに乗っかるカタチのユーティリティライブラリを作りたい」と言い換えることができます。

というわけで、今回は「良いライブラリを書く指針」について書きます。

ライブラリというものは、
「ユーザが表層に露出したインターフェースを使用して、何らかの目的を達成出来るようにするもの」
「ユーザが内部のコードに触ることができないため、内部のコードに触らなくても良いように書く」
「ユーザがやりたいことを記述できる汎用性を持たせる」
「ユーザは思いもよらないようなコードを書くので、出来る限り多くの状況をカバーした安全性を持たせる」
というような要素を満たすように設計されたもので、根本的にユーザコードとは書き方が異なってきます。

では、ライブラリとして機能を提供する際のたったひとつの注意点です。

  • ユーザが出来ることを、ひとつの機能につきたったひとつに制限する

「汎用性」と矛盾してると思いますか？そうではないです。クラスは「名前に準ずる機能を提供するもの」であり、「名前が機能そのものを表す」ものではありません。思いつく限りの汎用性のあるクラスはやはり、名前に準じた機能を提供しています。そして、その機能に「関係のないコンテキスト」を排除することで、「あらゆるものに」そのクラスの機能を提供することができているのです。今回は、機能を提供する際に「何を提供したかったのか」をはっきりさせていなかったため、どこをどう汎用化すれば良いのかがわからなくなっているのが悩みの原因だと推測します。クラス自体を「機能」と見なしてしまったため、その本質を正しく認識できなくなっているのです。

例として、今回は「EntityFrameworkでアクセス可能なDBで、テーブルからテーブルにデータを移動する」というたったひとつの機能を提供したいと決めるとします。それなら、例えば以下のようなAPIが存在すれば、どうでしょうか？ 

public static void MoveData<TDbContext, TA, TB>(
    this TDbContext context,
    Func<TDbContext, DbSet<TA>> from,
    Func<TDbContext, DbSet<TB>> to,
    Func<TA, TB> converter)
    where TDbContext : DbContext
    ;

APIだけでも、何を書けばどう動くのか、ある程度見えるのではないでしょうか。
「テーブルからテーブルにデータを移動したい」というたったひとつの目的にのみ使える代わりに、その目的に合致する操作であれば、どんな対象にでも適用できるように書けるはずです。（EntityFrameworkの細かい扱いに自信がないので、もっと良い設計があるかもしれません……）
そして、これ以外の機能を実装したいなら、それに合わせた「名前」を新たに決めて、そのたったひとつの目的を達成する機能（メソッド）を作ってしまえばいいのです。

その他ライブラリを作る--機能を提供する--時に気にすることというと、

  • 何かのフレームワークに乗っかるつもりならべったり乗っかる
  • 標準のインターフェース、依存するフレームワークのインターフェースをなるべく使用する
  • ユーザがコードを書く時に「面倒だ」と感じないようにする
  • ユーザにどう使ってほしいのか、ユーザがどのように使いたいと考えるかを常に意識する
  • シグネチャから何をするのか読み取れないメソッドは設計不足
  • public、private以外のaccessibilityも必要に応じて使っていく
  • ユーザはぶっ飛んだコードを書くと思って、出来る限りの状況を想定したコードを書く
  • ユーザが発生させる例外は決して握りつぶさない(rethrowで透過させてもいいし、何らかの失敗を通知する形があればそれでも良い)

みたいな感じですかね。

あんまり長々と書いても仕方ないので、サクッと締めます。
まずは「提供したい機能とその名前（シグネチャ）をはっきりと定める」ことから始めると良いと思います。

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

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

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

関連した質問

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

SNSアカウントでログイン

teratailアカウントでログイン

思考するエンジニアのためのQ&Aサイト「teratail」について詳しく知る