c# - この ASP.NET コンサルタントは自分が何をしているか知っていますか?

Question

当社の子会社の IT 部門は、コンサルティング会社に ASP.NET アプリケーションを作成してもらいました。現在、現在のユーザーが誰であるかを混同するという断続的な問題が発生しており、誤って Bob のデータの一部を Joe に表示することが知られています。

コンサルタントはトラブルシューティングのために呼び戻され、私たちは彼らの説明を聞くよう招待されました。2つのことが突き出ました。

最初に、コンサルタントリードが次の疑似コードを提供しました。

void MyFunction()
{
    Session["UserID"] = SomeProprietarySessionManagementLookup();
    Response.Redirect("SomeOtherPage.aspx");
}

彼は続けて、セッション変数の割り当ては非同期であると言いましたが、これは正しくないようです。ルックアップ関数への呼び出しが非同期的に何かを実行できることは確かですが、これは賢明ではないようです。

非同期性が主張されていることを考えると、リダイレクトの必然的な ThreadAbort 例外が発生する前に、セッション変数が割り当てられていなかったというのが彼の理論でした。この障害により、SomeOtherPage が正しいユーザーのデータを表示できなくなりました。

次に、彼が推奨するコーディングのベストプラクティスの例を示しました。書くのではなく：

int MyFunction(int x, int x)
{
    try 
    {
        return x / y; 
    }
    catch(Exception ex)
    {
        // log it
        throw;
    }
}

彼が推奨したテクニックは次のとおりです。

  int MyFunction(int x, int y, out bool isSuccessful)
  {
    isSuccessful = false;

    if (y == 0)
        return 0;

    isSuccessful = true;

    return x / y;
  }

これは確かに機能し、状況によってはパフォーマンスの観点から改善される可能性があります。

しかし、これらの議論やその他の論点から、このチームは技術に精通していないように思えました。

意見？

score 19 · Accepted Answer

経験則: コンサルタントが自分のしていることを知っているかどうかを尋ねる必要がある場合、彼はおそらく知らないでしょう ;)

そして、私はここに同意する傾向があります。明らかにあなたは多くを提供していませんが、彼らはそれほど有能ではないようです。

score 14 · Accepted Answer

私は同意します。これらの人はかなり無能に見えます。

（ところで、「SomeProprietarySessionManagementLookup」で静的データを使用しているかどうかを確認します。これを見ました-数か月前に継承したプロジェクトで説明したとおりの動作でした。私たちはついにそれを見ました...そして、それを書いた人たちと顔を合わせることができたらいいのにと思いました... )

score 12 · Accepted Answer

コンサルタントが、ユーザーを追跡し、正しいデータを正しいユーザーにのみ表示できるはずのアプリケーションを作成したにもかかわらず、それが行われない場合は、明らかに何かが間違っています。優れたコンサルタントは、問題を見つけて修正します。悪いコンサルタントは、それは非同期性だと言うでしょう。

score 8 · Accepted Answer

非同期部分では、成功/失敗を示すコールバックなしで非同期呼び出しを隠しているセッションのインデクサーセッターが実際に進行中の割り当てである場合にのみ、真になる可能性があります。これは恐ろしい設計上の選択のように思われ、フレームワークのコアクラスのように見えるので、そうなる可能性は非常に低いと思います。

通常、非同期呼び出しにはコールバックを指定する方法があるため、結果が何であるか、または操作が成功したかどうかを判断できます。セッションのドキュメントは、実際に非同期呼び出しを隠しているかどうかについてはかなり明確なはずですが、そうです...コンサルタントが彼が話していることを知っているようには見えません...

セッションインデクサーに割り当てられているメソッド呼び出しを非同期にすることはできません。非同期で値を取得するには、コールバックを使用する必要があるためです。、内部的に非同期呼び出しが存在する可能性がありますが、メソッドの呼び出し元はそれを同期として認識します。そのため、たとえばメソッドが内部的に Web サービスを非同期的に呼び出す場合は関係ありません)。

2番目の点については、これははるかに優れていると思います。基本的に同じ機能を維持します。

int MyFunction(int x, int y)
{
    if (y == 0)
    {
        // log it
        throw new DivideByZeroException("Divide by zero attempted!");
    }

    return x / y; 
}

score 5 · Accepted Answer

最初の点については、それは確かに奇妙に思えます。

2 番目のものでは、0 による除算を回避しようとするのが合理的です。これは完全に回避可能であり、回避は簡単です。ただし、out パラメーターを使用して成功を示すのは、int.TryParse や DateTime.TryParseExact など、呼び出し元が引数が妥当かどうかを簡単に判断できない場合にのみ妥当です。それでも、通常、戻り値は成功/失敗であり、out パラメーターはメソッドの結果です。

score 4 · Accepted Answer

Asp.net セッションは、組み込みプロバイダーを使用している場合、誤って他の誰かのセッションを提供することはありません。SomeProprietarySessionManagementLookup()原因である可能性が高く、悪い値を返しているか、機能していません。

Session["UserID"] = SomeProprietarySessionManagementLookup();

まず、非同期の SomeProprietarySessionManagementLookup() からの戻り値の割り当ては機能しません。コンサルタントのコードはおそらく次のようになります。

public void SomeProprietarySessionManagementLookup()
{
    // do some async lookup
    Action<object> d = delegate(object val)
    {
        LookupSession(); // long running thing that looks up the user.
        Session["UserID"] = 1234; // Setting session manually
    };

    d.BeginInvoke(null,null,null);               
}

コンサルタントは完全にBSでいっぱいではありませんが、バグのあるコードを書いています。Response.Redirect() は ThreadAbort をスローし、独自のメソッドが非同期の場合、asp.netは、asp.net 自体がセッションを保存する前に、非同期メソッドがセッションに書き戻すまで待機することを知りません。これがおそらく、機能する場合と機能しない場合がある理由です。

asp.net セッションがインプロセスの場合、それらのコードは機能する可能性がありますが、状態サーバーまたは db サーバーは機能しません。タイミング依存です。

以下をテストしました。開発ではステートサーバーを使用しています。メインスレッドが終了する前にセッションが書き込まれるため、このコードは機能します。

Action<object> d = delegate(object val)
{
    System.Threading.Thread.Sleep(1000);  // waits a little
    Session["rubbish"] = DateTime.Now;
};

d.BeginInvoke(null, null, null);
System.Threading.Thread.Sleep(5000);      // waits a lot

object stuff = Session["rubbish"];
if( stuff == null ) stuff = "not there";
divStuff.InnerHtml = Convert.ToString(stuff);

次のコードスニペットは機能しません。これは、非同期メソッドがセッション値を設定するまでに、セッションが既に状態サーバーに保存されているためです。

Action<object> d = delegate(object val)
{
    System.Threading.Thread.Sleep(5000);  // waits a lot
    Session["rubbish"] = DateTime.Now;
};

d.BeginInvoke(null, null, null);

// wait removed - ends immediately.
object stuff = Session["rubbish"];
if( stuff == null ) stuff = "not there";
divStuff.InnerHtml = Convert.ToString(stuff);

最初のステップは、パフォーマンストリックがまったく機能しなかったため、コンサルタントがコードを同期化することです。それで問題が解決する場合は、コンサルタントに非同期プログラミングデザインパターンを使用して適切に実装してもらいます。

score 2 · Accepted Answer

私は彼に部分的に同意します.(高価な)例外をキャッチするよりも、yをゼロでチェックする方が間違いなく良いです. out bool isSuccessful は、私には本当に時代遅れに思えますが、何でも構いません。

re: 非同期セッション ID の馬鹿騒ぎ -- 本当かもしれないし、そうでないかもしれませんが、コンサルタントが隠蔽のために煙を吹いているように聞こえます。

score 1 · Accepted Answer

この男は彼が何をしているのか知りません。明らかな犯人はここにあります：

Session["UserID"] = SomeProprietarySessionManagementLookup();

score 1 · Accepted Answer

コーディの経験則は完全に正しいです。あなたが尋ねる必要がある場合、彼はおそらくそうしません。

ポイント2のように思えますが、明らかに間違っています。.NET の標準では、メソッドが失敗した場合は例外をスローする必要があると説明されていますが、これはオリジナルに近いようです。コンサルタントの提案ではありません。例外が正確かつ具体的に失敗を説明していると仮定します。

score 1 · Accepted Answer

コンサルタントが最初にコードを作成しましたよね? そして、それは機能しません。すでにかなりの汚れが付いていると思います。

非同期の回答はBSのように聞こえますが、その中に何かがあるかもしれません. おそらく、彼らは適切な解決策と、彼ら自身が作成した問題を説明する疑似コードを提供しました。問題の表現よりも、解決策で彼らを判断したくなるでしょう。彼らの理解に欠陥があると、彼らの新しいソリューションも機能しません。そうすれば、彼らがばかだとわかるでしょう。（実際、コードの他の領域で同様の証明がすでにあるかどうかを確認するために周りを見てください）

もう 1 つはコードスタイルの問題です。それに対処する方法はたくさんあります。私は個人的にはそのスタイルが好きではありませんが、それが適している状況もあります。

score 1 · Accepted Answer

彼らは非同期で間違っています。

割り当てが行われ、ページがリダイレクトされます。関数は非同期で何かを開始して戻ることができます (そして、独自の方法でセッションを変更することも考えられます)。

0 または NULL または空の文字列などをそのように処理する必要がある特定のビジネスケースでない限り、低レベルのコードや高レベルの関数でさえ、その防御的なコーディングスタイルでは間違っています。常に成功し (成功フラグは厄介なコードの匂いです)、例外ではありません。例外は例外です。関数の呼び出し元を甘やかすことによって、このような動作をマスクしたくはありません。物事を早期にキャッチし、例外をスローします。マグワイアはこれをライティングソリッドコードで、またはマコーネルはコードコンプリートで取り上げたと思います。とにかく臭い。

score 0 · Accepted Answer

あなたのコンサルタントは、エラー処理の例外ではなくステータス変数を使用することを提案していると思いますか？同意しません。戻り値のエラーチェックを行うのを忘れたり、怠惰になったりする頻度はどれくらいですか？また、合格/不合格変数は有益ではありません。整数x/yが大きすぎる、またはxがNaNであるなど、ゼロ除算以外にも問題が発生する可能性があります。問題が発生した場合、ステータス変数は問題が発生したことを通知できませんが、例外は通知できます。例外は例外的な場合であり、ゼロ除算またはNaNは間違いなく例外的なケースです。

score 0 · Accepted Answer

コンサルタントがASP.NETアプリケーションをサーバーに展開した場合は、コンパイルされていない形式で展開されている可能性があります。つまり、大量の*.csファイルが浮かんでいる可能性があります。

あなたが見つけることができるのがそれらのコンパイルされた.NETアセンブリ（DLLとEXE）だけである場合でも、それらをある程度読みやすいソースコードに逆コンパイルすることができるはずです。コードを調べてみると、独自のルックアップコードで静的変数を使用していることがわかります。そうすれば、上司に見せるための非常に具体的なものができあがります。

score 0 · Accepted Answer

この回答ストリーム全体は、典型的なプログラマーの態度でいっぱいです。Joelの「絶対にやるべきでないこと」の記事を思い出します（最初から書き直してください）。バグがあることを除いて、システムについては何も知りません。誰かがオンラインでコードを投稿しました。未知数が非常に多いので、「この男は自分が何をしているのかわからない」と言うのはばかげています。

score 0 · Accepted Answer

SomeProprietarySessionManagementLookup（）;の場合非同期割り当てを実行している場合は、次のようになります。

SomeProprietarySessionManagementLookup(Session["UserID"]);

コードが結果をSession["UserID"]に割り当てているという事実は、非同期であるとは想定されておらず、Response.Redirectが呼び出される前に結果を取得する必要があることを示しています。結果が計算される前にSomeProprietarySessionManagementLookupが返される場合は、とにかく設計上の欠陥があります。

例外をスローするか、outパラメータを使用するかは、意見と状況の問題であり、実際には、どのような方法でも、豆の山にはなりません。例外のパフォーマンスへの影響が問題になるには、関数を何度も呼び出す必要がありますが、これはおそらくそれ自体が問題になるでしょう。

score 0 · Accepted Answer

セッション物も可能です。これは間違いなくバグですが、次の読み取りの後に、使用しているカスタムセッション状態プロバイダーに書き込みが到着する可能性があります。セッション状態プロバイダー API は、この種のことを防ぐためにロックに対応していますが、実装者がそれをすべて無視した場合、コンサルタントは真実を語っている可能性があります。

2番目の問題もちょっと有効です。これはかなり慣用的なものではありません - int.TryParse のようなもののわずかに逆のバージョンであり、多くの例外をスローすることによって引き起こされるパフォーマンスの問題を回避するために存在します。しかし、そのコードを大量に呼び出していない限り、顕著な違いが生じる可能性は低いです (たとえば、ページあたりのデータベースクエリが 1 つ少ないなどと比較して)。それは確かに、デフォルトで行うべきことではありません。

score 0 · Accepted Answer

コンサルタントに頼るのではなく、サービスを提供した人に頼ることもできます。コンサルタントも採用マネージャーも完璧ではありません。しかし、結局のところ、あなたが取るべき本当の方向性は非常に明確です。欠点を見つけようとするのではなく、協力して解決策を見つけることにエネルギーを費やすべきです。誰かが自分の役割と責任にどれだけ熟練していても、必ず不足があります。無能なパターンがあると判断した場合は、今後別のリソースに移行することを選択できますが、責任を負わせることで歴史上単一の問題が解決されたことはありません.

score 0 · Accepted Answer

かなり奇妙。2番目の項目では、より速い場合とそうでない場合があります。ただし、確かに同じ機能ではありません。

score 0 · Accepted Answer

典型的な「コンサルタント」ボロック:

問題は、 SomeProprietarySessionManagementLookup が行っていることです
例外は、スローされた場合にのみコストがかかります。を恐れる必要はありませんが、スローは例外的try..catchな状況でのみ発生する必要があります。variableがゼロであってはならない場合は、 anが適切です。y ArgumentOutOfRangeException

score 0 · Accepted Answer

私はジョン・ルディに同意しなければなりません。私の直感は、問題が SomeProprietarySessionManagementLookup() にあることを教えてくれます。

..そしてあなたのコンサルタントは自分自身に自信がないように聞こえます.

score 0 · Accepted Answer

非同期ではないセッションに保存します。その関数が非同期でない限り、それは真実ではありません。それでも、BeginCall を呼び出しておらず、完了時に何かを呼び出す必要がないため、次のコード行は Session 行が完了するまで実行されません。

2 番目のステートメントについては、それを使用することはできますが、ベストプラクティスとは言えず、注意すべき点がいくつかあります。例外をスローするコストを節約できますが、ゼロ除算ではなくゼロ除算をしようとしていることを知りたくありませんか?

決して堅実な提案ではないと思います。

score -1 · Accepted Answer

2 番目の点については、ここでは例外を使用しません。例外は、例外的なケースのために予約されています。
ただし、ゼロによる除算は確かに (少なくとも数学では) ゼロに等しくないため、これはケース固有です。

c# - この ASP.NET コンサルタントは自分が何をしているか知っていますか?

22 に答える 22

Related

Reference