これは効率的ですか、より良い方法でカプセル化を使用すべきですか？

Question

ここにコードがあります。この記事の最後にある質問をご覧ください。

public partial class myClass : Other.Class 
    { 
     long check1parameter = CurrentSession.CurrentFile.ID; 

     protected override void EnquiryLoaded(object sender, System.EventArgs e) 
     { 
      disableFields(); 
     } 
     private void disableFields() 
     { 
      if (checkEverything()) { 
       EnquiryForm.GetControl("Status").Enabled = true; 
      } 
     } 

     public bool check1_method(long check1parameter) { 
      bool Check1 = false; 
      string stringToCheck = check1parameter.ToString(); 
      if (stringToCheck.Contains("something")) { 
        Check1 = true; 
       } 
      return Check1; 
     } 

     public bool checkEverything() { 
      bool roleCheck = CurrentSession.CurrentUser.IsInRoles("RequiredRole"); 
      bool check1 = check1_method(check1parameter); 
      bool checkEverything = false; 
      if (roleCheck && check1) { 
       checkEverything = true; 
      } 
      return checkEverything; 
     } 
     //other methods 
    } 

コードは、誰かが役割を持っていることを確認すると、文字列が情報のビットが含まれていることをして、フィールドを無効にすることです。私は、これを実際のコードから簡略化して、キーポイントを概説しました。これらの単純なチェックを実行してフィールドを無効にすることだけが意図されていますが、後で展開できるように、これらのタスクの個別のメソッドを作成することをお勧めします。

long check1parameterがその位置に定義されていると、オブジェクト参照エラーが発生します。それはcheck1_method()にあり、正しく動作しましたが、可能ならば一度宣言して、複数の領域にわたって使用したいと思っています。

また、パラメータ\変数を内部で宣言するのではなくcheck1_methodに渡したいとします。この部分クラスのすべてのメソッドでcheck1parameterを利用できるようにするにはどうすればよいでしょうか？何らかの形でOther.Classにリンクされている別のクラスを指します。

私の主な質問は - これをできるだけ効率的にするにはどうすればいいですかここでpublicの代わりにprivateを使用する必要がありますか？私はまだC＃で​​非常に新しく、まだカプセル化を理解していないので、簡単に私に行ってください！ :)

Answer 1

myClassあなたが別のファイルに実装し続けるつもりでない限り、部分的に宣言する必要はありません。

それらを削除することができ、単純なif文を使用して、たとえば次のように書くことができます：不要なboolsを宣言するから自分を救うために

public partial class myClass : Other.Class 
    { 
     long check1parameter = CurrentSession.CurrentFile.ID; 

     protected override void EnquiryLoaded(object sender, System.EventArgs e) 
     { 
      disableFields(); 
     } 
     private void disableFields() 
     { 
      EnquiryForm.GetControl("Status").Enabled = checkEverything(); 
     } 

     public bool check1_method(long check1parameter) { 
      return check1parameter.ToString().Contains("something"); 
     } 

     public bool checkEverything() { 
      bool roleCheck = CurrentSession.CurrentUser.IsInRoles("RequiredRole"); 
      bool check1 = check1_method(check1parameter); 

      return (roleCheck && check1); 
     } 
     //other methods 
    } 

。それを超えると、より少ない行で読みやすさを犠牲にすることになります。

publicとprivateの場合は、クラスの外部からアクセスする必要がある場合を除き、常にprivateを指定することをお勧めします。一見すると、disableFields()はおそらく公開され、check1_method()とcheckEverything()はプライベートであるべきです。

EDIT：check1parameterがmyClassにグローバルにインスタンス化されている場合 また、あなたは、あなたが提供されたコードがOKに見えるcheck1_methods()

Answer 2

にパラメータとしてそれを渡す必要はありません。私はいくつかの変更、主にコードの美学を作りました。主なものは、2つのチェックメソッドをプロパティにすることです。

public partial class myClass : Other.Class 
{ 
    long check1parameter = CurrentSession.CurrentFile.ID; 

    protected override void EnquiryLoaded(object sender, System.EventArgs e) 
    { 
     disableFields(); 
    } 

    private void disableFields() 
    { 
     if (checkEverything) 
     { 
      EnquiryForm.GetControl("Status").Enabled = true; 
     } 
    } 

    // the parameter name was the same as a variable in the class 
    // renamed to avoid confusion 
    public bool check1_method 
    { 
     get {return check1parameter.ToString().Contains("something");} 
    } 

    public bool checkEverything 
    { 
     get { return CurrentSession.CurrentUser.IsInRoles("RequiredRole") 
      && check1_method; } 
    } 
    //other methods 
}