none
ASP.NET MVC で SQL文の塊を静的クラスで宣言することについて RRS feed

  • 質問

  • お世話になります。
    最近、Webシステム開発のプロジェクトに入ったのですが、(自分自身では)すごく不思議だと思うコードを見つけたので、皆さまのご意見をお聞きしたくて投稿いたしました。

    開発言語:C#
    ASP.NET MVC  .NET Framework 4.5 の開発環境です。

    SQL文字列を作成するメソッドだけを持った xxxSqlModel.cs というファイルがあります。内容は、以下のようなものです。

     public class  U001_SqlModel
     {
          public static string SelectData()
          {
                StringBuilder sb = new StringBulder(SIZE_MAX);
                sb.Append("SELECT ");
                sb.Append("   MS.PRODUCT_CD ");
                sb.Append("  ,MS.STAFF_NO ");
                    :
                    :
                    :
                sb.Append("; ");
                return sb.ToString();
           }
           public static string InsertData()
           {
                  ※上記と同じ造り
           }
           :
           :
           :
     }

    上記のようなクラスが複数あり、静的メソッドとして宣言されています。
    静的メソッドは static な領域に可能される。しかし、文字列はヒープ領域に格納されると思っています。
    そのため、これらのメソッドに static を付けるメリットがあるようには思えません。
    むしろ、protected なクラス宣言をし、インスタンス化して使った方が良いように思います。
    実装者にそのことを話そうと思うのですが、その前にこちらのフォーラムでお聞きしたいと思い、投稿いたしました。
    上記のようなSQLの文字列を返却するだけのメソッドを静的メソッドとして定義することに、何かメリットはあるのでしょうか。

    よろしくお願いいたします。


    2017年12月21日 7:46

回答

  • > 実装者にそのことを話そうと思うのですが、その前にこちらのフォーラムでお聞きしたいと思い、投稿いたしました。

    質問する順番が逆だと思いますよ。

    実装者さんには、ここに書いてあること以外は何も知り得ない第三者には分からない、何らかの目的・意図があったのかもしれません。

    例えば、以下のような形で DataTable から動的にクエリを作る必要があれば、同じようなことはするかも知れません。static でなければならないのも何か訳があるのかも。

    ACE OleDb で Excel のブック作成
    http://surferonwww.info/BlogEngine/post/2012/01/26/Creating-Excel-workbook-by-using-ACE-OleDb-provider.aspx

    それを知らずして議論はできません。まず、あなたが実装者さんにあなたの疑問をぶつけてください。話はそれからだと思います。

    • 回答としてマーク JOSIE39 2017年12月26日 2:23
    2017年12月21日 9:05
  • SQL発行のほんの一時必要なクラスなら、SQL文取得の時だけインスタンス化するような使い方でよいような気がいたしました。

    なちゃさんも指摘されていますが基本的な知識が欠落しています。

    非staticが必要な時だけインスタンスを生成できるのに対し、staticは不用意に常時インスタンスを生成・保持しているとお考えであればそれは間違っています。staticはそもそもインスタンスを生成不要だからインスタンスを生成せずに済むものです。
    つまりほんの一時であれ不要なインスタンスは生成しないに越したことはありません。実装者さんがstaticを選択した理由もその点にあります。

    なお、近年ではO/Rマッパーと呼ばれるSQL文を自動生成するツールを使用するのが一般的となっています。.NET FrameworkでもEntity Frameworkというライブラリが標準で組み込まれています。

    SurferOnWwwさんへ:
    個人的な意見ですが、質問する順番が逆とは思いません。質問者さんが公開フォーラムで広く意見を募ろうと判断したからこその投稿であり、特定の実装者の意見に偏るよりもいいことだと思います。

    • 回答としてマーク JOSIE39 2017年12月26日 2:23
    2017年12月21日 13:36
  • > 投稿のあと、実装者に「なぜ、このクラスは 静的メソッドにしているのでしょうか」と聞いてみました。
    > 回答は「何となく、サンプルがそうなっていたから」という理由でした。

    実装者さんとして理由は明確で、その理由に基づき組織としてそいうコーディングルールがあるなど、ここで第三者と議論するような話ではないと想像していましたが・・・

    「何となく・・・」だとすると、実際のところはサンプルを作った人に聞かないと分からないと思いますが、想像を膨らませると以下のようなことではないかと思います。

    (1) 長いクエリを一行で書くと可読性が落ちる。

    (2) と言って、"SELECT aaa, bbb, ccc ... ", "FROM Table1 ", "WHERE xxx = @XXX ..." というように複数行に分けて、それらを連結するような方法を取ると性能上の問題があると思った。

    (実際は連結された文字列がインターンプールに格納され、実行時に連結するわけではないので性能上の問題はないと思うのですが)

    (3) なので StringBuilder を使って文字列を連結するメソッドを定義してクエリの文字列を生成するようにした。

    (4) クエリの文字列を得るという目的に、メソッドを含むクラスを初期化してインスタンスを生成する必要は全くない(というか、むしろすべきでないと思います)ので static メソッドとした。

    質問者さんは、インスタンスメソッドにするとメモリの節約などメリットがあると考えられたのだと思いますが、それは誤解だと思います。

    ところで、ASP.NET MVC アプリとのことですけど、例えば以下の記事のように Entity Framework と Linq to Entity をベースにスキャフォールディング機能を使ってアプリを作るような場合、SQL を書くということはなくなると思うのですが、いかがですか?

    スキャフォールディング機能
    http://surferonwww.info/BlogEngine/post/2017/07/23/creating-controller-and-view-in-mvc-using-scaffolding-function.aspx

    ASP.NET Web Forms アプリの場合でも、SqlDataSource などを使ってウィザードベースで作る場合、クエリビルダで SQL 文を作って、その文字列をソースコードに埋め込むという手法を取る場合がほとんどのはずです。

    質問者さんのケース、

    > 上記のような SQL文字列を取得するようなソースファイルが100本近くあり、その理由は画面数に比例しているらしいのです。

    という点に質問者さんのアプリ固有の特殊性があるのかもしれませんが・・・


    • 編集済み SurferOnWww 2017年12月22日 6:11 一部訂正
    • 回答としてマーク JOSIE39 2017年12月26日 2:23
    2017年12月22日 5:54
  • ちなみにSQL文を保持するのであれば、前述した通り、メソッドなど必要なく、以下のようにすれば十分だと思いますよ。

    public static readonly string SelectSql = @"
        select  a, b, c
        from table1
        where a > @a
    ";


    ★良い回答には回答済みマークを付けよう! MVP - .NET  http://d.hatena.ne.jp/trapemiya/

    2017年12月22日 6:19
    モデレータ

すべての返信

  • 静的メソッドは static な領域に可能される。しかし、文字列はヒープ領域に格納されると思っています。そのため、これらのメソッドに static を付けるメリットがあるようには思えません。むしろ、protected なクラス宣言をし、インスタンス化して使った方が良いように思います。
    逆に質問ですが、JOSIE39さんの案のように非staticにした場合、文字列はどこに格納されるとお考えなのでしょうか?
    2017年12月21日 8:05
  • お世話になります。

    非 static にした場合、文字列はヒープ領域と思います。(staticな領域もヒープ領域の一部と思っています。)
    文字列のAppendしかやらないメソッドを static宣言する事を不思議に思いました。
    随分前に、下記サイトの記事を参考にさせて頂いたのですが、共有が必要なメンバや使用頻度の高いメソッドなどを static にするのは、確かにあり得るんだろうなと思うのですが、SQL発行のほんの一時必要になるだけのメソッドに static を付ける事のメリットがよく分かりません。

    参考サイト:https://blogs.msdn.microsoft.com/nakama/2008/12/17/453/
    参考サイト:https://blogs.msdn.microsoft.com/nakama/2008/12/24/443/

    SQL発行のほんの一時必要なクラスなら、SQL文取得の時だけインスタンス化するような使い方でよいような気がいたしました。

    • 編集済み JOSIE39 2017年12月21日 8:50
    2017年12月21日 8:50
  • どこに格納されるというより、インスタンスを作成する必要があるかないかという視点で見られてみてはいかがでしょうか?
    インスタンス毎に異なる状態になる可能性がなければ、インスタンスは1つで良いことになります。しかも、そのインスタンスがどんな場合でも同じであれば、インスタンスである必然性もなくなります。

    ちなみに、StringBuilderを使う必要はなく、@で始まる逐語的文字列を使えば十分であるように思います。これであれば、例えばSQL Server Management Systemでクエリをテストした後、ほとんどそのままそのSQL文をコピペすることができます。


    ★良い回答には回答済みマークを付けよう! MVP - .NET  http://d.hatena.ne.jp/trapemiya/

    2017年12月21日 9:02
    モデレータ
  • 何か壮大な勘違いというか認識違いがあるようにも思えます。

    >SQL発行のほんの一時必要なクラスなら、SQL文取得の時だけインスタンス化するような使い方でよいような気がいたしました。

    SQL文取得のほんのひと時の、インスタンスに依存のないメソッドを実行するために、わざわざ一時インスタンス化するほうが、「わざわざ」に思えます。

    もしかしてstaticメソッドの場合、staticに何かが占有され続けていて、インスタンスメソッドの場合は、インスタンスを作成した時しかメソッドは何も占有していない、とかのように思っているでしょうか?


    • 編集済み なちゃ 2017年12月21日 9:03
    2017年12月21日 9:02
  • > 実装者にそのことを話そうと思うのですが、その前にこちらのフォーラムでお聞きしたいと思い、投稿いたしました。

    質問する順番が逆だと思いますよ。

    実装者さんには、ここに書いてあること以外は何も知り得ない第三者には分からない、何らかの目的・意図があったのかもしれません。

    例えば、以下のような形で DataTable から動的にクエリを作る必要があれば、同じようなことはするかも知れません。static でなければならないのも何か訳があるのかも。

    ACE OleDb で Excel のブック作成
    http://surferonwww.info/BlogEngine/post/2012/01/26/Creating-Excel-workbook-by-using-ACE-OleDb-provider.aspx

    それを知らずして議論はできません。まず、あなたが実装者さんにあなたの疑問をぶつけてください。話はそれからだと思います。

    • 回答としてマーク JOSIE39 2017年12月26日 2:23
    2017年12月21日 9:05
  • SQL発行のほんの一時必要なクラスなら、SQL文取得の時だけインスタンス化するような使い方でよいような気がいたしました。

    なちゃさんも指摘されていますが基本的な知識が欠落しています。

    非staticが必要な時だけインスタンスを生成できるのに対し、staticは不用意に常時インスタンスを生成・保持しているとお考えであればそれは間違っています。staticはそもそもインスタンスを生成不要だからインスタンスを生成せずに済むものです。
    つまりほんの一時であれ不要なインスタンスは生成しないに越したことはありません。実装者さんがstaticを選択した理由もその点にあります。

    なお、近年ではO/Rマッパーと呼ばれるSQL文を自動生成するツールを使用するのが一般的となっています。.NET FrameworkでもEntity Frameworkというライブラリが標準で組み込まれています。

    SurferOnWwwさんへ:
    個人的な意見ですが、質問する順番が逆とは思いません。質問者さんが公開フォーラムで広く意見を募ろうと判断したからこその投稿であり、特定の実装者の意見に偏るよりもいいことだと思います。

    • 回答としてマーク JOSIE39 2017年12月26日 2:23
    2017年12月21日 13:36
  • 佐祐理さん>

    > 個人的な意見ですが、質問する順番が逆とは思いません。

    私は逆だと思ってますよ。

    > 特定の実装者の意見に偏るよりもいいことだと思います。

    とのことですが、「特定の実装者の意見」はまだ聞いてないです。

    2017年12月21日 14:52
  • 皆さま、たくさんのご意見をありがとうございます。

    先に実装者と話した方がよかったのかもしれません。
    自身も知識がないため、実装者と先に話ても暗雲が立ち込めると思い、先にご意見をお聞きした次第です。
    投稿のあと、実装者に「なぜ、このクラスは 静的メソッドにしているのでしょうか」と聞いてみました。
    回答は「何となく、サンプルがそうなっていたから」という理由でした。
    その回答で、ますます気になったので、もう少し、皆さまからのご意見や本来こうする事が望ましいなどのアドバイスを頂けませんでしょうか。
    よろしくお願いいたします。

    もう少し追記しますと、先に記載の、以下のような静的メソッドが宣言された csファイルが100本近くあります。

    public class  U001_SqlModel
     {
          public static string SelectData()
          {
                StringBuilder sb = new StringBulder(SIZE_MAX);
                    :
                    :
                sb.Append("AND PRODUCT_CD = @product_cd ");
                sb.Append("AND STAFF_NO   = @staff_no;");
                    :
                sb.Append("; ");
                return sb.ToString();
           }
           public static string InsertData()
           {
                  ※上記と同じ造り
           }
           :
           :
     }

    静的メソッドの呼び出し元は、文字列を取得した後、次のような処理が書かれています。

     string sql = U001_SqlModel.SelectData();
     SqlCommand cmd = new SqlCommand(sql, connection);
     cmd.Parameters.AddWithValue("@product_cd", productCD);
      :
     command.ExecuteNonQuery();

    疑問に思ったのは、sql文字列を取得するメソッドがすべて静的メソッドになっている点です。
    sql文字列が static 宣言されたstring 型であれ、上記のような " " で囲まれた文字列であれ、ヒープに格納され、アプリケーション寿命(と言っていいのでしょうか) で存在すると思っています。
    疑問に感じたのは、メソッドの方です。
    メソッドを静的メソッドにする必要があるのかという点に疑問を感じました。

    上記のような SQL文字列を取得するようなソースファイルが100本近くあり、その理由は画面数に比例しているらしいのです。
    そして、開発途中なので、これからまだまだ増える予定だそうです。
    100本のうち、このSQL文を取得する静的メソッドが頻繁に呼び出されるものは20本程度。残りの80本はログインユーザーの権限により表示画面が絞られるので、各静的メソッドが呼び出される頻度は1日に1回あるかないかだそうです。

    この頻度を考えた時に、静的メソッドにして存在させておく必要はないのではないか、要る時にインスタンス化して終われば破棄でもいいのでは?と疑問を感じたため、投稿いたしました。
    寿命や生成と破棄の事などを、つい考えた時、このメソッドにおいては、インスタンス化の方でもいいような気がいたしました。
    この先、まだまだ本数が増える。けれど使用頻度は少ないという事からも、要るときにインスタンス化して使う方向でもよいように思った次第です。

    記載のようなメソッドの場合でも 静的メソッドとして使う方向もありなのでしょうか。
    皆さまのご意見をお聞かせください。よろしくお願いいたします。


    • 編集済み JOSIE39 2017年12月22日 3:43
    2017年12月22日 3:41
  • 自身も知識がないため、実装者と先に話ても暗雲が立ち込めると思い、先にご意見をお聞きした次第です。

    それで構わないと考えます。少なくともSurferOnWwwさんに「実装者に質問してから出直す」よう命じる権利はありません。

    疑問に思ったのは、sql文字列を取得するメソッドがすべて静的メソッドになっている点です。sql文字列が static 宣言されたstring 型であれ、上記のような " " で囲まれた文字列であれ、ヒープに格納され、アプリケーション寿命(と言っていいのでしょうか) で存在すると思っています。

    最初の質問「文字列はどこに格納されるとお考えなのでしょうか?」はまさしくこの点に関する誤解の有無を確認するためものです。

    staticであれ、非staticであれソースコード中に埋め込まれている文字列はすべからく実行ファイル(EXEかDLL)に格納されており、それがプログラム起動時にメモリに読み込まれています。もちろんASP.NETでもこの点は同じです。
    つまりメソッドを非staticに変更したところで文字列の寿命はstaticと何ら変わりありません。

    2017年12月22日 4:40
  • この頻度を考えた時に、静的メソッドにして存在させておく必要はないのではないか、要る時にインスタンス化して終われば破棄でもいいのでは?と疑問を感じたため、投稿いたしました。
    寿命や生成と破棄の事などを、つい考えた時、このメソッドにおいては、インスタンス化の方でもいいような気がいたしました。

    インスタンスメソッドであれば、インスタンスメソッドがインスタンス化されるのではなく、そのメソッドが属しているオブジェクトがインスタンス化されます。メソッドだけのためにメモリが確保されるわけではありません。メソッドはコードの一部であり、スタックやヒープ、スタティック領域にメモリが確保されてロードされるわけではありません。
    インスタンスメソッドにしても静的メソッドにしても、プログラムのコードとしてメモリにロードされます。静的メソッドであれば、そのまま実行できますが、インスタンスメソッドであればそこからオブジェクトを生成してようやくメソッドが実行できることになります。
    インスタンスメソッドにする理由が無い時に、そのメソッドが属するクラスをインスタンス化し、ヒープにメモリを確保した上でそのメソッドを実行する方が、わざわざということになります。

    なちゃさんも指摘されていますが、おそらく、インスタンス化しないものはメモリ上に存在しないという前提で考えられているのだと思いますが、そういうわけではありません。


    ★良い回答には回答済みマークを付けよう! MVP - .NET  http://d.hatena.ne.jp/trapemiya/

    2017年12月22日 5:37
    モデレータ
  • > 投稿のあと、実装者に「なぜ、このクラスは 静的メソッドにしているのでしょうか」と聞いてみました。
    > 回答は「何となく、サンプルがそうなっていたから」という理由でした。

    実装者さんとして理由は明確で、その理由に基づき組織としてそいうコーディングルールがあるなど、ここで第三者と議論するような話ではないと想像していましたが・・・

    「何となく・・・」だとすると、実際のところはサンプルを作った人に聞かないと分からないと思いますが、想像を膨らませると以下のようなことではないかと思います。

    (1) 長いクエリを一行で書くと可読性が落ちる。

    (2) と言って、"SELECT aaa, bbb, ccc ... ", "FROM Table1 ", "WHERE xxx = @XXX ..." というように複数行に分けて、それらを連結するような方法を取ると性能上の問題があると思った。

    (実際は連結された文字列がインターンプールに格納され、実行時に連結するわけではないので性能上の問題はないと思うのですが)

    (3) なので StringBuilder を使って文字列を連結するメソッドを定義してクエリの文字列を生成するようにした。

    (4) クエリの文字列を得るという目的に、メソッドを含むクラスを初期化してインスタンスを生成する必要は全くない(というか、むしろすべきでないと思います)ので static メソッドとした。

    質問者さんは、インスタンスメソッドにするとメモリの節約などメリットがあると考えられたのだと思いますが、それは誤解だと思います。

    ところで、ASP.NET MVC アプリとのことですけど、例えば以下の記事のように Entity Framework と Linq to Entity をベースにスキャフォールディング機能を使ってアプリを作るような場合、SQL を書くということはなくなると思うのですが、いかがですか?

    スキャフォールディング機能
    http://surferonwww.info/BlogEngine/post/2017/07/23/creating-controller-and-view-in-mvc-using-scaffolding-function.aspx

    ASP.NET Web Forms アプリの場合でも、SqlDataSource などを使ってウィザードベースで作る場合、クエリビルダで SQL 文を作って、その文字列をソースコードに埋め込むという手法を取る場合がほとんどのはずです。

    質問者さんのケース、

    > 上記のような SQL文字列を取得するようなソースファイルが100本近くあり、その理由は画面数に比例しているらしいのです。

    という点に質問者さんのアプリ固有の特殊性があるのかもしれませんが・・・


    • 編集済み SurferOnWww 2017年12月22日 6:11 一部訂正
    • 回答としてマーク JOSIE39 2017年12月26日 2:23
    2017年12月22日 5:54
  • ちなみにSQL文を保持するのであれば、前述した通り、メソッドなど必要なく、以下のようにすれば十分だと思いますよ。

    public static readonly string SelectSql = @"
        select  a, b, c
        from table1
        where a > @a
    ";


    ★良い回答には回答済みマークを付けよう! MVP - .NET  http://d.hatena.ne.jp/trapemiya/

    2017年12月22日 6:19
    モデレータ
  • > ちなみにSQL文を保持するのであれば、前述した通り、メソッドなど必要なく、以下のようにすれば十分だと思いますよ。

    自分もそう思います。

    でも、sql query stringbuilder などをキーワードにググってみると、StringBuilder を使うべきだと言っている人もちらほらおられるようです。

    その流れからの組織のコーディングルールがあって、それに従わざるを得ないということもあるのかもしれないとは思いますが。

    2017年12月22日 6:46
  • > ちなみにSQL文を保持するのであれば、前述した通り、メソッドなど必要なく、以下のようにすれば十分だと思いますよ。

    自分もそう思います。

    でも、sql query stringbuilder などをキーワードにググってみると、StringBuilder を使うべきだと言っている人もちらほらおられるようです。

    その流れからの組織のコーディングルールがあって、それに従わざるを得ないということもあるのかもしれないとは思いますが。

    確かに検索してみると出てきますね。

    組織のコーディングルールでやむを得ない場合はともかく、検索で出てきたのいくつか読んでみると、残念ながら単に知識が足りていないだけっぽいですね…

    2017年12月22日 8:50
  • 皆さま、ご意見ありがとうございます。

    文字列の扱いについては、一定のコーディングルールがあり、その中に「StringBuilder でSQL文字列を生成する」という条件のものもあります。
    ただ、作成するクラスのスコープについては、実装者まかせなので「なぜ、静的メソッドに?」と疑問を感じた事から始まりました。

    ・文字列であれ、コードであれ、すべてEXEやDLL内に存在する。
    ・文字列はヒープ領域に、コードはコード領域に存在している。
    ・静的メンバや静的メソッドはアプリ寿命で実行時から存在する。
    ・インスタンス化は、new 呼ばれた時にクラスオブジェクト生成(ヒープの確保など)後、使用可能。


    インスタンス化する方向でもいいのではないかと思ったのは、現時点で静的メソッドの100中 80本は1日1回呼ばれるかどうかのものであるという点です。
    1日1回、Select()が呼ばれるか呼ばれないかレベルならば、要るときにインスタンス化(メモリ確保)すればいいと思ったのです。
    使われもしないコードがアプリ寿命で実体化(こう表現してもいいものでしょうか)している事に疑問を感じました。
    「ただ何となくstatic。楽でいいから」いう理由で、何でも staticにしそうな勢いなので、う~ん?そうなのか?と疑問です。

    > 質問者さんは、インスタンスメソッドにするとメモリの節約などメリットがあると考えられたのだと思いますが、それは誤解だと思います。

    SurferOnWwwさんのおっしゃる「メリット」があるのか無いのかすら、今ひとつ分からないというのが本心です。

    インスタンス化はインスタンス化でコストがかかるし、実体化する機会の差と思えば、staticのままでもいいのかとも思います。
    しかし、使われる頻度が限りなく少ないメソッドで、それも文字列を返却するだけのメソッドが、静的メソッドとして実行時からずっと存在する事に、やはりまだ疑問を感じています。
    返却する文字列は、すでにヒープに存在していて、return sb.ToString() するだけのメソッドを静的メソッドにする必要性があるのか。
    アプリ寿命でずっと存在していて。その先もどんどん増えていくというのかが引っかかっります。


    話がそれるかもしれませんが、多分、私が気にした事の理由の根底にあるのは、実体化する機会の差の違いと寿命のような気がします。
    短絡的な書き方と思いますが、
      実体化の差:static(初回の実行時からメモリにロード済み)とインスタンス化(new されたときにロード)
      寿命   :staticはアプリ寿命、インスタンスはインスタンス化された場所による(ガベージ対象)

    と思っているので、今回の文字列を返却するだけのメソッドに対して過敏反応したのかもしれません。
    そして、SurferOnWwwさんがその点を「メモリの節約などのメリット」と表現してくださったのかもしれません。


    話は戻りますが、

    > ちなみにSQL文を保持するのであれば、前述した通り、メソッドなど必要なく、以下のようにすれば十分だと思いますよ。

    そうですね。trapemiyaのおっしゃるよう通りの書き方で十分ですね。
    代替えコードを見ると、私が何に引っかっているのか明確になった気がします。

    余計に静的メソッドにする必要があるのかと疑問が沸きます。この先どんどん増やすのか、ほんとにこれでいいのかと疑問が沸きます。


    > ところで、ASP.NET MVC アプリとのことですけど、例えば以下の記事のように Entity Framework と Linq to Entity をベースにスキャフォールディング機能を使ってアプリを作るような場合、SQL を書くということはなくなると思うのですが、いかがですか?

    > スキャフォールディング機能
    > http://surferonwww.info/BlogEngine/post/2017/07/23/creating-controller-and-view-in-mvc-using-scaffolding-function.aspx
    >
    > ASP.NET Web Forms アプリの場合でも、SqlDataSource などを使ってウィザードベースで作る場合、クエリビルダで SQL 文を作って、その文字列をソースコードに埋め込むという手法を取る場合がほとんどのはずです。

    ありがとうございます。このようなやり方もあるという事をメンバーに展開したいと思います。

    どう言葉で説明すればいいか難しいですが、プロジェクトの作成自体は、ASP.NET MVC でVisual Studio で見た構成も ModelゃController と分かれておりますか、中身は昔ながらのやり方(と思っているのは私の理解不足からかもしれません)です。
    SQL文も Management Studioで作成して、動く事を確認したら上述のコードに書き直すというやり方です。
    ASP.NET MVCでやると決めた時に、新しい事を学ぶなら今なので、Entity FrameworkとかLINQとかにトライしてみましょうと提案もしましたが、納期に間に合わないとの事で「O/RマッピングとかEntity Frameworkを使うのは、スキルが足りないから後回し」と意味の分からない説明で却下され、現状に至っています。
    1年前の事です。その資産をベースに今のプロジェクトが作成されているので、古い書き方と思われるコードも継承されています。

    自身は、コードのスコープとかが、つい気になってしまいます。私もそのプロジェクトで実装担当ですが、人のコードをコピペして作成する度に「?」マークが飛びだすので、皆さまからのご意見やアドバイスを頂けて有難いです。

    上述のような文字列しか返却しないメソッドが public static メソッドとして100個近く存在し、この先も増えていくという状況に出くわした時、皆さまならどうされますか?
    このようなメソッドでも静的メソッドで続けてもいいものでしょうか。

    今どきそんな書き方しないから、想定外と言われるかもしれまんが、皆さまなら、どうされますか?
    trapemiyaさんが仰るような書き方に、ささやかな改修は有りのような気がするのですが・・・。

    皆さま、再度で申し訳ありませんが、ご意見を頂けますよう、よろしくお願いいたします。


    2017年12月22日 11:56
  • > このようなメソッドでも静的メソッドで続けてもいいものでしょうか。

    それについては、私が、先のレスで、

    > (4) クエリの文字列を得るという目的に、メソッドを含むクラスを初期化してインスタンスを生成する必要は全くない(というか、むしろすべきでないと思います)ので static メソッドとした。

    と言いました。少なくともそこのところは 100% 正しいと思っています。

    他の回答者の皆さんも、そこのところは同じ意見と理解していますけど、いかがですか?>他の回答者の皆様

    疑問なのは (2) だけです。そこを突き詰めていくと、そもそも StringBuilder を使ってメソッドでクエリの文字列を組み立てるのはどうかという話になるのですが、そこはちょっと置いときましょう。

    質問者さんは仕事で製品としてのアプリを開発しているのですよね? であれば、上のような基本的なことについては組織内でしかるべき教育がされるはずでは?

    なぜなら、製品品質に影響のある業務に携わる要員はしかるべくスキルを持っていなければ、組織は製品品質を担保できないから。それゆえ、要員に必要なスキルレベルに達するべく教育を施すのは組織の責任ではないのですか?

    もし、質問者さんが質問者さんの属する組織内で聞ける状況にない、教育を受けられる状況にないということでここで聞いているとすると、それは製品に品質責任のある組織としてはいかがなものかと思うのですが・・・

    2017年12月22日 13:46
  • 根本的な勘違いを正してからでないと、正しく物事を理解できないですよ。

    -----

    staticメソッドはずっとメモリ上に確保されている。

    インスタンスメソッドはインスタンス化された時だけメモリ上に確保される。

    -----

    というような感じの理解は、根本的に間違っています。

    インスタンスメソッドもstaticメソッドも、一度そのメソッドが呼び出されたら(一応呼び出されたらという表現にしておきます)、アプリ終了までずっとそのメソッド本体とでもいうべきものはメモリ上に確保されたままです。

    staticでもインスタンスでも同じです。

    そしてインスタンスメソッドは、加えて、オブジェクトのインスタンス化が必要です。

    インスタンスメソッドは、staticメソッドと同等の領域はそもそもずっと確保され続けるうえに、追加でオブジェクトのインスタンス化が必要です。

    2017年12月23日 2:54
  • 返却する文字列は、すでにヒープに存在していて、return sb.ToString() するだけのメソッドを静的メソッドにする必要性があるのか。
    アプリ寿命でずっと存在していて。その先もどんどん増えていくというのかが引っかかっります。

    静的メソッド内のローカル変数にもスコープがあります。よって、静的メソッドを抜ければ、そのローカル変数は破棄されますので、返却する文字列がずっと残っているということはありません。

    上述のような文字列しか返却しないメソッドが public static メソッドとして100個近く存在し、この先も増えていくという状況に出くわした時、皆さまならどうされますか?
    このようなメソッドでも静的メソッドで続けてもいいものでしょうか。

    基本的には全てストアドプロシージャで記述します。私がSQL文をプログラム側で書くのは、TableAdapterにおいて、自動的にUpdateメソッドを生成したい時ぐらいです。
    データベースにおけるデータの処理は、T-SQLで記述した方が柔軟かつ簡単に書けます。ただ、これにはT-SQL等、データベース側の知識が必要になるので、その知識が浅くても済むようにプログラム側でいろいろな仕組みが導入されてきています。
    ただ、本当はT-SQLにも精通されると、プログラム側で複雑なことをしなくても実現できることが多くなるので、できればT-SQLにも精通されることをお勧めします。
    例えば、データの整合性をチェックする場合、プログラムだとループで回してなどと処理しなければいけないのに、T-SQLだと1文で書けたりします。なのでT-SQLのスキルがあるのとないのとでは、大違いだなぁというのが私の感想です。
    一度には無理かもしれませんが、そういう方向性があるということを意識して勉強を続けられると、きっと損はないと思います。

    ★良い回答には回答済みマークを付けよう! MVP - .NET  http://d.hatena.ne.jp/trapemiya/

    2017年12月23日 6:34
    モデレータ
  • 返信が遅くなって、申し訳ありません。
    皆様、ご意見をありがとうございます。

    > インスタンスメソッドもstaticメソッドも、一度そのメソッドが呼び出されたら(一応呼び出されたらという表現にしておきます)、アプリ終了までずっとそのメソッド本体とでもいうべきものはメモリ上に確保されたままです。

    なちゃ様、ありがとうございます。
    その点については、理解しているつもりでした。一度でも呼び出されたら、ガページ対象になるまでずっとメモリ上に確保されたままという理解でおりました。
    インスタンス生成の場所(クラスメンバかローカルメンバかみたいな・・・)によって、インスタンスの寿命が変わる。static は完全にアプリ寿命と思っておりましたので、極端な話、インスタンス生成にしておけば、スレッドが不要になった時点でそれは破棄されるから、そっちの方がいいかと思っておりました。

    今回のお問合せで、IIS、アプリケーションプール、.NET の関係とかが、今ひとつ分かっていないため、まともな質問になっていなかった気がします。すみません。


    > なぜなら、製品品質に影響のある業務に携わる要員はしかるべくスキルを持っていなければ、組織は製品品質を担保できないから。それゆえ、要員に必要なスキルレベルに達するべく教育を施すのは組織の責任ではないのですか?
    > もし、質問者さんが質問者さんの属する組織内で聞ける状況にない、教育を受けられる状況にないということでここで聞いているとすると、それは製品に品質責任のある組織としてはいかがなものかと思うのですが・・・

    SurferOnWww さんのご想像通りです。
    上司に開発経験者がおらず(営業からの上司ばかり)、技術的な相談をする相手はいません。
    できるSEとはとかプロマネとは等の、どちらかといえば根性論的な教育にはお金をかけますが、プログラミングの技術的な事については「自力で」を要求される会社です。
    会社の理念に「人を大切に」とありますが、たぶん「お気に入りを大切に」の間違いという事も理解しています(笑)
    とにかく動けばいい的な発想なので、このフォーラムに投稿したような内容は「どう書いても動くなら、どっちでもいいじゃないか。細かい事にこだわるな」で一刀両断です。
    周りの開発者もそういう対応で、とりあえず動くからいい的な感じですが、そこに呑み込まれたくない気持ち(大げさですね)で、投稿させて頂いています。


    > StringBuilder を使ってメソッドでクエリの文字列を組み立てるのはどうかという話になるのですが、そこはちょっと置いときましょう。

    投稿時には、この点について、疑問を持ったことには、間違いありません。
    文字列はすでにヒープに確保されているのに、メソッドの目的がそのヒープに確保されている文字列を連結して返却するだけ。それが static ?というのが疑問の始まりと思います。


    そこから皆さんから、static/インタンス の事やサンプルやクエリの動的生成、スキャンフォールディング機能などの情報サイトなどを教えて頂く事ができました。
    皆様、どうもありがとうございました。

    教えて頂いた事は、同プロジェクトメンバーと共有しました。

    StringBuilder を使ってメソッドクエリ文字列を組み立てるのは止めようと思います。今日以降の実装は、クエリ文字列は readonly string の定義でいく方向で進めます。

    社内の実装担当者同士で話し合ういい機会になりました。皆様、どうもありがとうございました。


    • 編集済み JOSIE39 2017年12月26日 3:01
    2017年12月26日 2:22
  • > インスタンスメソッドもstaticメソッドも、一度そのメソッドが呼び出されたら(一応呼び出されたらという表現にしておきます)、アプリ終了までずっとそのメソッド本体とでもいうべきものはメモリ上に確保されたままです。

    なちゃ様、ありがとうございます。
    その点については、理解しているつもりでした。一度でも呼び出されたら、ガページ対象になるまでずっとメモリ上に確保されたままという理解でおりました。
    インスタンス生成の場所(クラスメンバかローカルメンバかみたいな・・・)によって、インスタンスの寿命が変わる。static は完全にアプリ寿命と思っておりましたので、極端な話、インスタンス生成にしておけば、スレッドが不要になった時点でそれは破棄されるから、そっちの方がいいかと思っておりました。


    最終的にわかっているのか、まだ誤解があるのかがちょっと判断つかないので、念のためここだけ。

    ガベージ対象になるまでずっと保持ではなくて、アプリが終了するまでずっと保持です。

    staticでもインスタンスでも同じなので、インスタンスにしても何も節約にならないということです(むしろ追加で必要)。

    2017年12月26日 6:03
  • > 文字列はすでにヒープに確保されているのに、メソッドの目的がそのヒープに確保されている文字列を連結して返却するだけ。それが static ?というのが疑問の始まりと思います。

    その疑問は解消された、即ち、「ヒープに確保されている文字列を連結して返却するだけ」であれば static メソッドにすべき(インスタンスを生成する必要は全くない)ということは納得されたのでしょうか?

    何となく、まだ納得されてないような気がしますが・・・


    > StringBuilder を使ってメソッドクエリ文字列を組み立てるのは止めようと思います。

    結局はそれが正解だと思います。

    書き方は trapemiya さんが書かれた「@で始まる逐語的文字列」を使うのがよさそうです。


    以下は余計な話かもしれませんが、ついでに書いておきます。

    ***** 佐祐理さんの指摘を受けて訂正しました。*****

    複数行に分ける場合、例えば以下のコードの s2, s3 のようにするのは止めた方がよさそうです。

    string s0 = "SELECT aaa, bbb, ccc FROM Table1 WHERE ddd = @ddd";
    
    string s1 = "SELECT aaa, bbb, ccc " +
                "FROM Table1 " +
                "WHERE ddd = @ddd";
    
    string s2 = "SELECT aaa, bbb, ccc ";
    s2 += "FROM Table1 ";
    s2 += "WHERE eee = @eee";
    
    string s3 = new StringBuilder().
                Append("SELECT aaa, bbb, ccc ").
                Append("FROM Table1 ").
                Append("WHERE fff = @fff").
                ToString();
    
    string t1 = new StringBuilder().Append("WHERE ddd ").Append("= @ddd").ToString();
    string t2 = new StringBuilder().Append("WHERE eee ").Append("= @eee").ToString();
    string t3 = new StringBuilder().Append("WHERE fff ").Append("= @fff").ToString();
    
    Console.WriteLine("Is s1 the same reference as s0?: {0}", (object)s1 == (object)s0);
    Console.WriteLine("'{0}' is {1}", t1, (string.IsInterned(t1) == null) ? "not interned" : "interned");
    Console.WriteLine("'{0}' is {1}", t2, (string.IsInterned(t2) == null) ? "not interned" : "interned");
    Console.WriteLine("'{0}' is {1}", t3, (string.IsInterned(t3) == null) ? "not interned" : "interned");
    
    // 結果は:
    // Is s1 the same reference as s0?: True
    // 'WHERE ddd = @ddd' is not interned
    // 'WHERE eee = @eee' is interned
    // 'WHERE fff = @fff' is interned

    文字列 "SELECT aaa, bbb, ccc FROM Table1 WHERE ddd = @ddd" はインターンプールに格納され、s0 と s1 は同じオブジェクトを指します。(即ち、(object)s1 == (object)s0 は true になります)

    s1 の右辺のように分けて + で連結するのはよさそうです。

    しかし、s2, s3 の場合、結果から、右辺の個々の文字列のオブジェクトも作られインターンプールに格納されるという無駄なことが行われるようです。

    ***** ここまで *****

    ちなみに、インターンプールというのは、文字列を key に、ヒープ上の String オブジェクトのアドレスを value に持つハッシュテーブルのようなものだそうです。詳しくは以下の記事の開設を見てください。

    String.Intern メソッド
    https://msdn.microsoft.com/ja-jp/library/system.string.intern(v=vs.100).aspx

    寿命についてもインターンプール特有のものがあるそうです。詳しくは上の記事の「パフォーマンスに関する考慮事項」のセクションを見てください。




    • 編集済み SurferOnWww 2017年12月26日 9:12 訂正
    2017年12月26日 6:51
  • そこのところは良いのですが、str2 の右辺の個々の文字列のオブジェクトも作られインターンプールに格納されるという無駄なことが行われるようです。(例えば、string.IsInterned("WHERE xxx = @xxx") は null にならず文字列への参照が返ってきます)

    墓穴を掘っておられるかと。string.IsInterned("文字列リテラル") が "文字列リテラル" を返すのは当たり前です。string.IsInterned(string.Concat("WHERE xxx", " = @xxx")) 等のように文字列を実行時に生成した上で評価しなければ意味がありません。

    というわけで文字列リテラル同士の + 結合はコンパイル時に結合され、実行時には結合結果の文字列のみが残ります。個々の文字列はコンパイルの過程で削除されます。

    2017年12月26日 7:22
  • なちゃ様

    ガベージ対象になるまでずっと保持ではなくて、アプリが終了するまでずっと保持です。
    >
    staticでもインスタンスでも同じなので、インスタンスにしても何も節約にならないということです(むしろ追加で必要)。

    念押し、ありがとうございます。
    上記のように覚えておきます。



    2017年12月26日 8:50
  • > string.IsInterned("文字列リテラル") が "文字列リテラル" を返すのは当たり前です。

    string.IsInterned("文字列リテラル") と書けば "文字列リテラル" オブジェクトがヒープに生成されて、なおかつインターンプルに格納されることになるので、これは全く意味がなかったようです。

    さらに、文字列の + での連結の場合は、連結される個々の文字列はインターンプルには格納されない(その前に、個々の文字列オブジェクトそのものが生成されない?)ようですね。

    訂正しておきます。指摘をありがとうございました。

    2017年12月26日 8:51
  • SurferOnWww 様

    >その疑問は解消された、即ち、「ヒープに確保されている文字列を連結して返却するだけ」であれば static メソッドにすべき(インスタンスを生成する必要は全くない)ということは納得されたのでしょうか?

    はい、「そもそもヒープにいるんだから」の視点に認識間違いがあった事を理解いたしました。
    アプリケーションプールやらスレッドやら、.NETのガベージやら、いつの間にか
    「コスト」について、思い込んでいたように思います。
    「ヒープに確保されている文字列を連結して返却するだけ」であれば static メソッドにすべき(インスタンスを生成する必要は全くない)は、よくわかりました。

    加えて、情報もありがとうございます。

    2017年12月26日 9:00
  • 文字列については、+で結合されたリテラル文字列はコンパイル時に単一文字列として連結され、実行時にはそのままヒープにロードされる。
    文字列変数との結合の場合は、実行時に連結が行われるので、連続したメモリ空き領域が足りず、連結できない場合は、メモリの再確保の処理が走るということですね?

    あ~、だから、StringBuilder を使っていたのかもしれません。しかし、ソースのコピペが進み、いつの間にか、リテラル文字列ばかりの連結でも StringBuilde を使っていたのかもしれません。それは、それで見直そうと思います。

    ありがとうございました。
    2017年12月26日 9:06
  • やり取りを読む限りJOSIE39さんの「ヒープ」に対する理解も怪しく感じられます。

    ヒープにもいろいろあり、また文字列も格納場所がいろいろとあります。特にソースコード中に ”” 形式で埋め込まれているリテラル文字列はプログラムコードと同じ扱いをされています。具体的にはGCで管理はされていますが、プログラムコードと同時に読み込まれプログラム実行中には解放処理が行われることはありません。
    これは広義のヒープではありますが、JOSIE39さんの思い描かれているヒープとは異なるはずです。

    なお、StringBuilderやString.Concatなどで実行時に生成された文字列はこれとは別の扱いがされています。GCで適切に管理され使用されなくなった時点で解放されます。

    最初の質問「文字列はどこに格納されるとお考えなのでしょうか?」はやはり質問者さんの誤解を端的に聞き出していたと思います。

    2017年12月26日 20:50
  • この念押しはこの投稿で説明した内容そのままですね。
    2017年12月26日 20:52
  • この念押しはこの投稿で説明した内容そのままですね。

    余談ですが、実をいうと私が書いたのは、プログラムコード側(JITコンパイルが済んだ実行コードなど)がメモリ上にどのように残っているかという話でした。

    ※メソッドのコード(メソッド本体)も、staticだろうとインスタンスだろうと、メモリ上にずっと残り続けるという話

    今回の話は、stringオブジェクトそのものの観点の他に、メソッド自体がどのように存在しているか、という観点がありそうだったので、私はメソッド自体がどのように存在している、という観点で書いていました。

    --追記

    コード中の文字列は、.NETの場合はどちらかというと単にインターンプールに保存された普通の文字列インスタンスと同じ扱いに近い感覚なので、単に私の受け取り方のニュアンスが違うだけかもしれません。

    • 編集済み なちゃ 2017年12月27日 0:57
    2017年12月27日 0:50
  • 文字列変数との結合の場合は、実行時に連結が行われるので、連続したメモリ空き領域が足りず、連結できない場合は、メモリの再確保の処理が走るということですね?


    なんかいろいろ細かいところが気になり…

    .NETでは文字列は変更不可なので、連結は必ず新しい文字列領域がマネージヒープ上に確保されます。

    ※マネージヒープ(やGC)の動きはまたいろいろと複雑なので一言では言えませんが、なーんとなく誤解してそうな違和感が…

    2017年12月27日 1:02