c# - C＃アンチパターン

Question

簡単に言うと、 Javaのアンチパターンは不可欠なリソースだと思います。初心者からプロまで。私はまだC＃用にこのようなものを見つけていません。それで、私はこの質問をコミュニティwikiとして開き、これに関する知識を共有するようにみんなを招待します。私はC＃を初めて使用するので、これに強く興味がありますが、いくつかのアンチパターンから始めることはできません：/

これが私がC＃に特に当てはまり、他の言語には当てはまらないと思う答えです。

これらをコピーして貼り付けただけです！これらについてもコメントを見てみることを検討してください。

投げ`NullReferenceException`

間違った例外をスローする：

if (FooLicenceKeyHolder == null)
    throw new NullReferenceException();

プロパティとパブリック変数

クラスのパブリック変数（代わりにプロパティを使用してください）。

クラスが単純なデータ転送オブジェクトでない限り。

ブール値が単なる慣例ではなく、実際の型であることを理解していない

if (myBooleanVariable == true)
{
    ...
}

または、さらに良い

if (myBooleanVariable != false)
{
    ...
}

Cこのような構造は、ブール値の概念が単なる慣例であった開発者や開発者によってよく使用されC++ます（0 == false、その他はtrue）。これは、C＃または実際のブール値を持つ他の言語では必要ありません（または望ましくありません）。

使用する`using()`

using必要に応じて利用しない：

object variable;
variable.close(); //Old code, use IDisposable if available.
variable.Dispose(); //Same as close.  Avoid if possible use the using() { } pattern.
variable = null; //1. in release optimised away.  2. C# is GC so this doesn't do what was intended anyway.

score 62 · Accepted Answer

例外を誤って再スローします。例外を再スローするには:

try
{
    // do some stuff here
}
catch (Exception ex)
{
    throw ex;  // INCORRECT
    throw;     // CORRECT
    throw new Exception("There was an error"); // INCORRECT
    throw new Exception("There was an error", ex); // CORRECT
}

score 40 · Accepted Answer

40

GC.Collect()ガベージコレクターを信頼する代わりに収集します。

于 2009-10-07T05:25:41.033 に答える

score 30 · Accepted Answer

Java と C# の両方で、これは 1 つの方法で多すぎます...

if(something == true){
  somethingelse = true;
}

それも持っている場合はボーナスポイント付き

else{
  somethingelse = false;
}

score 25 · Accepted Answer

25

using Microsoft.SharePoint;

'言っ途切れる

于 2009-10-09T12:35:42.727 に答える

score 22 · Accepted Answer

次のコードをよく見かけます。

if (i==3)
       return true;
else
       return false;

次のようにする必要があります。

       return (i==3);

score 17 · Accepted Answer

デメテルの法則を侮辱する:

a.PropertyA.PropertyC.PropertyB.PropertyE.PropertyA = 
     b.PropertyC.PropertyE.PropertyA;

score 17 · Accepted Answer

投げるNullReferenceException：

if (FooLicenceKeyHolder == null)
    throw new NullReferenceException();

score 16 · Accepted Answer

これは本当です。私は自分の目でそれを見ました。

public object GetNull()
{
     return null;
}

これは実際にアプリで使用されており、それに対応するストアドプロシージャ、nullを返すsp_GetNullもありました。

それが私の一日になりました。

spは古典的なaspサイトに使用されたと思います..結果セットと関係があります。.netの1つは、コードを.netに「変換」するというアイデアでした。

score 14 · Accepted Answer

int foo = 100;
int bar = int.Parse(foo.ToString());

または、より一般的なケース：

object foo = 100;
int bar = int.Parse(foo.ToString());

score 12 · Accepted Answer

私たちのプロジェクトでこれを見つけて、ほとんど椅子を壊しました...

DateTime date = new DateTime(DateTime.Today.Year, 
                             DateTime.Today.Month, 
                             DateTime.Today.Day);

score 11 · Accepted Answer

この種の var-abuse に出くわすことがよくあります。

var ok = Bar();

またはさらに良い：

var i = AnyThing();

そのように var を使用しても意味がなく、何も得られません。コードを追跡するのが難しくなるだけです。

score 10 · Accepted Answer

デリゲート呼び出し前の null テストの欠如。
null チェックと try/catch を使用したキャストで「as」を使用するタイミングと方法がわからない。
catch ブロック内での「例外のスロー」と「スロー」。
StringBuilder を使用する代わりに、多数の文字列をインスタンス化します。
ブロックを使用する深いネスティング。

score 10 · Accepted Answer

bool が単なる慣例ではなく実数であることを理解していない

if (myBooleanVariable == true)
{
    ...
}

または、さらに良い

if (myBooleanVariable != false)
{
    ...
}

このような構成は、ブール値の概念が単なる慣例 (0 == false、それ以外はすべて true) である開発者によってよく使用されCます。C++これは、実際のブール値を持つ C# やその他の言語では必要ありません (または望ましくありません)。

更新: 明確にするために、最後の段落を言い換えました。

score 9 · Accepted Answer

クラスのパブリック変数（代わりにプロパティを使用してください）。

クラスが単純なデータ転送オブジェクトでない限り。

議論と明確化については、以下のコメントを参照してください。

score 8 · Accepted Answer

これは実際に見たことがある。

bool isAvailable = CheckIfAvailable();
if (isAvailable.Equals(true))
{ 
   //Do Something
}

isAvailable == trueアンチパターンのハンズダウンを打ち負かします！
これを超アンチパターンに！

score 6 · Accepted Answer

object variable;
variable.close(); //Old code, use IDisposable if available.
variable.Dispose(); //Same as close.  Avoid if possible use the using() { } pattern.
variable = null; //1. in release optimised away.  2. C# is GC so this doesn't do what was intended anyway.

score 6 · Accepted Answer

6

プライベート自動実装プロパティ:

private Boolean MenuExtended { get; set; }

于 2009-10-07T05:32:06.543 に答える

score 6 · Accepted Answer

各メソッドの先頭ですべてのローカル変数を宣言して初期化するのはとても見苦しいです!

void Foo()
{
    string message;
    int i, j, x, y;
    DateTime date;

    // Code
}

score 6 · Accepted Answer

2 つの文字列のアンチパターン
アンチパターン # 1
文字列の null または空のチェック

//Bad
if( myString == null || myString == "" )
OR
if( myString == null || myString.Length == 0 )

//Good
string.IsNullOrEmpty(myString)

アンチパターン # 2 (.NET 4.0 のみ)
文字列の null または空または空白をチェックする

//Bad
if( myString == null || myString == "" || myString.Trim() == "")

//Good
string.IsNullOrWhiteSpace(myString)

score 5 · Accepted Answer

不必要なキャスト (コンパイラを信頼してください):

foreach (UserControl view in workspace.SmartParts)
{
  UserControl userControl = (UserControl)view;
  views.Add(userControl);
}

score 5 · Accepted Answer

if(data != null)
{
  variable = data;
}
else
{
  variable = new Data();
}

次のように書くほうがよい

variable = (data != null) ? data : new Data();

そしてさらに良いのは次のように書かれたものです

variable = data ?? new Data();

最後のコードリストは .NET 2.0 以降で機能します

score 4 · Accepted Answer

訛りのある話し方はいつも私をひきつけました。

C++ プログラマー:

if (1 == variable) { }

C# では、と入力するとコンパイラエラーが発生するため、失敗するif (1 = variable)心配をせずに、意図したとおりにコードを記述できます。

score 4 · Accepted Answer

3 進数を使用しないことは、C# への変換が時々行うことです。

分かりますか：

private string foo = string.Empty;
if(someCondition)
  foo = "fapfapfap";
else
  foo = "squishsquishsquish";

それ以外の：

private string foo  = someCondition ? "fapfapfap" : "squishsquishsquish";

score 3 · Accepted Answer

変更されたクロージャへのアクセス

foreach (string list in lists)
{
        Button btn = new Button();
        btn.Click += new EventHandler(delegate { MessageBox.Show(list); });
}

（説明と修正についてはリンクを参照してください）

score 3 · Accepted Answer

文字列ビルダーの代わりに文字列連結を使用して、任意の数の文字列を連結する場合

例

foreach (string anItem in list)
    message = message + anItem;

score 2 · Accepted Answer

これは一般的と見なされますか？

public static main(string [] args)
{
  quit = false;
  do
  {
  try
  {
      // application runs here .. 
      quit = true;
  }catch { }
  }while(quit == false);
}

説明する方法はわかりませんが、誰かが例外をキャッチして、後で機能することを期待してコードを何度も再試行するようなものです。IOExceptionが発生した場合と同様に、IOExceptionが機能するまで何度も試行します。

score 2 · Accepted Answer

私が取り組んでいるプロジェクトには 50 個のクラスがあり、すべてが同じクラスから継承され、次のように定義されていました。

public void FormatZipCode(String zipCode) { ... }

それを親クラスに入れるか、ユーティリティクラスを横に置きます。ああ。

The Daily WTFを閲覧することを検討しましたか?

score 1 · Accepted Answer

.NETの主な問題は、多くの開発者がVB 6.0から来ているという事実、または（VB 6.0プログラマーが少なくとも十分に謙虚であるのに、彼らは何をすべきかを知っていると誤って信じているため、私の意見ではさらに悪い）という事実のようです。何か新しいことを学ぶために）Java /C++。

現代のパラダイムにあまりにも無知な人々、可能な限り最悪のC++スタイルで醜いP/Invokeでコードを塗りつぶしている人々。:-(

score 1 · Accepted Answer

1

すべてを実行したい、非常に複雑な'Page_Load'メソッド。

于 2009-10-07T05:24:16.397 に答える

score 1 · Accepted Answer

私が継承したシステムでこれを数回見つけました...

if(condition){
  some=code;
}
else
{
  //do nothing
}

およびその逆

if(condition){
  //do nothing
}
else
{
  some=code;
}

score 1 · Accepted Answer

単純に値を取得したり、安価な計算を行う以外の目的でプロパティを使用する。プロパティからデータベースにアクセスしている場合は、メソッド呼び出しに変更する必要があります。開発者は、メソッドの呼び出しにコストがかかることを期待していますが、プロパティからはコストがかかるとは考えていません。

score 1 · Accepted Answer

if (state == ((int)RowState.Active).ToString())
else if (state == ((int)RowState.NotActive).ToString())

state は、RowState 列挙型の値を含む文字列 post 値です。

最終的には、これが値に対してチェックするために使用する方法です。

score 1 · Accepted Answer

私は以前にこれを持っていました：

AnEnum e = AnEnum.Abc;
int i = (int)e;
// lots of code
AnEnum f = (AnEnum)Enum.Parse(i, typeof(AnEnum));

score 0 · Accepted Answer

プロパティをコーディングするときは、その使用法を考えずにゲッターとセッターを自動的に与えるだけです。多くの場合、get または set は使用されず、プロパティは読み取り (取得) 専用または書き込み (設定) 専用にする必要があります。

score 0 · Accepted Answer

最近ちょっとだけ見ました。

終わらないパラムチェーン

public string CreateJob(string siteNumber, string customer, string jobType, string description, string reference, string externalDoc, string enteredBy, DateTime enteredDateTime)
    {
        //recordtype = 0 for job
        //load assignments and phases set to false
        return Create(0, siteNumber, customer, jobType, description, reference, externalDoc, enteredBy, enteredDateTime, false, false);
    }

public string Create(int recordType, string siteNumber, string customer, string jobType, string description, string reference, string externalDoc, string enteredBy, DateTime enteredDateTime, bool loadAssignments, bool loadPhases)
{
    _vmdh.Fields.FieldByName("WDDOCTYPE").SetValue(recordType, false);
    _vmdh.Fields.FieldByName("NMDOCID").SetValue(-1, false);
    _vmdh.Init();           
        ....
        ...
        // And it keeps going
    }

フォームクローズで何が起こるのだろうか

 private void frmAddImages_FormClosing(object sender, FormClosingEventArgs e)
{
    if (DialogResult != DialogResult.OK)
    {
        if (IsDirty)
        {
            e.Cancel = !(MessageBox.Show("Are you sure that you want to exit without saving", "Form Not Saved", MessageBoxButtons.YesNo) == DialogResult.Yes);
        }
    }
    }

文字列型

switch (cbDateFilter.Text)
            {
                case "This Week":
                    dt = DateTime.Now;
                    while (dt.DayOfWeek != DayOfWeek.Monday) dt = dt.AddDays(-1); //find first day of week
                    dtFrom.Value = DateTime.Parse(dt.ToString("dd/MM/yyyy 00:00:00"));
                    dtTo.Value = DateTime.Parse(dt.AddDays(6).ToString("dd/MM/yyyy 23:59:59"));
                    break;

                case "This Month":
                    dt = DateTime.Now;
                    while (dt.Day != 1) dt = dt.AddDays(-1); // find first day of month
                    dtFrom.Value = DateTime.Parse(dt.ToString("dd/MM/yyyy 00:00:00"));
                    dtTo.Value = DateTime.Parse(dt.AddMonths(1).AddDays(-1).ToString("dd/MM/yyyy 23:59:59"));
                    break;

                case "This Quarter":
                    // if at end of Quarter then we need subtract -4 to get to priv Quarter
                    dt = DateTime.Now;
                    while (dt.Month != 7 &&
                        dt.Month != 10 &&
                        dt.Month != 1 &&
                        dt.Month != 4) dt = dt.AddMonths(-1); //find first month, fiscal year
                    while (dt.Day != 1) dt = dt.AddDays(-1); // find first day on month
                    dtFrom.Value = DateTime.Parse(dt.ToString("dd/MM/yyyy 00:00:00"));
                    dtTo.Value = DateTime.Parse(dt.AddMonths(3).AddDays(-1).ToString("dd/MM/yyyy 23:59:59"));
                    break;

score 0 · Accepted Answer

無知は至福です（フレームワークを知ってください）：

TimeSpan keyDays = new TimeSpan(Licence.LicenceExpiryDate.Ticks);
TimeSpan nowDays = new TimeSpan(System.DateTime.Now.Ticks);

int daysLeft = keyDays.Days - nowDays.Days;

score 0 · Accepted Answer

使用（悪い）

IEnumerable<Bar> foo = ...
if (foo.Count() > 0)
{
    ...
}

（良い）の代わりに

IEnumerable<Bar> foo = ...
if (foo.Any())
{
    ...
}

IEnumerable に何かが含まれているかどうかをテストします。 Count()コレクション全体をで列挙する必要がありますMoveNext()がAny()、呼び出す必要があるのはMoveNext()1 回だけです。

score -2 · Accepted Answer

おそらく怠惰のため、すべてのオブジェクト初期化子の過剰使用/乱用:

var person = new Person
{
    FirstName = "joe",
    (... lots of setters down here)
};

これは、すべてのフィールドを公開するのと同じくらい悪いことに気付かずに。オブジェクトを有効な状態に初期化する有効なコンストラクターを作成する際には、常に注意する必要があります。

c# - C＃アンチパターン

投げNullReferenceException

プロパティとパブリック変数

ブール値が単なる慣例ではなく、実際の型であることを理解していない

使用するusing()

38 に答える 38

Related

Reference

投げ`NullReferenceException`

使用する`using()`