2016-10-19 6 views
0

改善の余地があると感じている機能に取り組んでいます。C#デザイン関連のクエリ(リファクタリングの改善)

シナリオ:

私は、物理的な機器のボタンのさまざまな種類があります。各ボタンタイプには特定の機能があります(ボリュームアップやボリュームダウンなど)。

私は、特定のアクション(短押し、中押し、長押し)に応じて新しいボタンタイプの機能を実現するという課題を持っています。私は、他のボタンタイプのための機能を得るために使用されているアプローチに従っていました。

最終結果:

最終的な結果は、新しいボタン型クラスは、ボタンがサポートするすべての機能を返す必要があることをする必要があります。

実装:

私は "InstrumentButtonBase" 継承 "InstrumentButtonType4を" クラスを作成しました。

"InstrumentButtonType4" クラスボタンが利用可能であり、有効にする場合、最初に確認 こと、方法GetButtonTypeFunctionalities()を有しています。

次に、アクションに応じてボタン機能を追加します。

私は3つの動作が短押し、長押し、中押しです。

メインロジック: -

私はアクションのそれぞれの機器に保存されている現在の値を取得 "int型shortPress = _instrumenrData.GetValue(3);" と静的マッパーを呼び出します」指定された「shortPress」値にマップされた対応する機能を取得するために、「InstructionButtonFunctionalityMapper」と入力します。

私はそれがより良い方法

  1. AddFunctionalityForShortPress(に書き込むことができる場合は、これらの3つのメソッドを改善したい

    より良い方法でリファクタリングしたい)

  2. AddFunctionalityForMediumPress()
  3. AddFunctionalityForVeryLongPress()

これらの3つの方法のロジックは、より良い設計で記述できますか?何らかの理由でMapperを削除して、 の実装をよりシンプルに/フレキシブルにスケーラブルにすることはできますか?

以下のクラスコードを貼り付けました。 コンソールアプリケーションでコードをコンパイルできます。 あなたの専門家のアドバイスは、私がこれをより良い方法で学び、達成するのに本当に役立ちます。

public interface IInstrumentData 
{ 
    int GetValue(int location); 
} 

public abstract class InstrumentButtonBase 
{ 
    public abstract bool IsAvailable(); 
    public abstract bool IsEnable(); 

    public abstract IDictionary<ButtonType, IDictionary<ButtonAction, ButtonFunctionality>> GetButtonTypeFunctionalities(); 
} 


public class InstrumentButtonType4 : InstrumentButtonBase 
{ 
    private readonly IInstrumentData _instrumenrData; 

    public InstrumentButtonType4(IInstrumentData instrumenrData) 
    { 
     _instrumenrData = instrumenrData; 
    } 

    public override IDictionary<ButtonType, IDictionary<ButtonAction, ButtonFunctionality>> GetButtonTypeFunctionalities() 
    { 
     var instrumentMaps = new Dictionary<ButtonType, IDictionary<ButtonAction, ButtonFunctionality>>(); 
     var buttonFunctionality = new Dictionary<ButtonAction, ButtonFunctionality>(); 

     if (!IsAvailable() && !IsEnable()) 
     { 
      return instrumentMaps; 
     } 

     AddButtonFunctionality(buttonFunctionality); 
     instrumentMaps.Add(ButtonType.Volume, buttonFunctionality); 

     return instrumentMaps; 
    } 

    private void AddButtonFunctionality(Dictionary<ButtonAction, ButtonFunctionality> buttonFunctionality) 
    { 
     AddFunctionalityForShortPress(buttonFunctionality); 
     AddFunctionalityForMediumPress(buttonFunctionality); 
     AddFunctionalityForVeryLongPress(buttonFunctionality); 
    } 

    private void AddFunctionalityForVeryLongPress(Dictionary<ButtonAction, ButtonFunctionality> buttonFunctionality) 
    { 
     // Get v 
     int veryLongPress = _instrumenrData.GetValue(1); // gets the data from the instrument itself 

     // calls mapper to get the corresponding functionality 
     ButtonFunctionality functionality = InstructionButtonFunctionalityMapper.Mapper["InstrumentButtonType1" + veryLongPress]; 
     buttonFunctionality.Add(ButtonAction.ShortPress, functionality); 
    } 

    private void AddFunctionalityForMediumPress(Dictionary<ButtonAction, ButtonFunctionality> buttonFunctionality) 
    { 
     int mediumPress = _instrumenrData.GetValue(2); // gets the data from the instrument itself 

     // calls mapper to get the corresponding functionality 
     ButtonFunctionality functionality = InstructionButtonFunctionalityMapper.Mapper["InstrumentButtonType1" + mediumPress]; 
     buttonFunctionality.Add(ButtonAction.MediumPress, functionality); 
    } 

    private void AddFunctionalityForShortPress(Dictionary<ButtonAction, ButtonFunctionality> buttonFunctionality) 
    { 
     int shortPress = _instrumenrData.GetValue(3); // gets the data from the instrument itself 

     // calls mapper to get the corresponding functionality 
     ButtonFunctionality functionality = InstructionButtonFunctionalityMapper.Mapper["InstrumentButtonType1" + shortPress]; 
     buttonFunctionality.Add(ButtonAction.ShortPress, functionality); 
    } 

    public override bool IsAvailable() 
    { 
     // checks some logic and returns tru or false 
     return true; 
    } 

    public override bool IsEnable() 
    { 
     // checks some logic and returns tru or false 
     return true; 
    } 
} 

public static class InstructionButtonFunctionalityMapper 
{ 
    private static Dictionary<string, ButtonFunctionality> _mapper; 
    public static Dictionary<string, ButtonFunctionality> Mapper 
    { 
     get 
     { 
      return _mapper ?? (_mapper = FillMapper()); 
     } 
    } 

    private static Dictionary<string, ButtonFunctionality> FillMapper() 
    { 
     var mapper = new Dictionary<string, ButtonFunctionality> 
     { 
      {"InstrumentButtonType1" + "VeryLongPress" + "0", ButtonFunctionality.DoAction1}, 
      {"InstrumentButtonType1" + "VeryLongPress" + "1", ButtonFunctionality.DoAction2}, 
      {"InstrumentButtonType1" + "VeryLongPress" + "2", ButtonFunctionality.DoAction3}, 
      {"InstrumentButtonType1" + "0", ButtonFunctionality.ProgramDown}, 
      {"InstrumentButtonType1" + "1", ButtonFunctionality.ProgramUp}, 
      {"InstrumentButtonType1" + "2", ButtonFunctionality.VolumeDown}, 
      {"InstrumentButtonType1" + "3", ButtonFunctionality.VolumeUp}, 
      {"InstrumentButtonType1" + "4", ButtonFunctionality.DoAction1}, 
      {"InstrumentButtonType1" + "5", ButtonFunctionality.DoAction4}, 
      {"InstrumentButtonType1" + "6", ButtonFunctionality.DoAction5} 
     }; 


     return mapper; 
    } 
} 

public enum ButtonFunctionality 
{ 
    VolumeUp, 
    VolumeDown, 
    ProgramUp, 
    ProgramDown, 
    DoAction1, 
    DoAction2, 
    DoAction3, 
    DoAction4, 
    DoAction5 
} 

public enum ButtonAction 
{ 
    ShortPress, 
    MediumPress, 
    LongPress, 
} 

public enum ButtonType 
{ 
    PushButton, 
    Volume 
} 

ありがとうございます!

+4

このような質問をコードレビューサイトに投稿する方が良いかもしれません。 https://codereview.stackexchange.com/ –

答えて

1
  1. 1,2などの「マジックナンバー」を避けるようにしてください。代わりにenumまたは定数を使用してください。
  2. ShortPress = 0 ...のような明示的な列挙型の値を使用してください。
  3. "InstrumentButtonType1"は一定である必要があります。
  4. AddFunctionalityForMediumPress、AddFunctionalityForShortPress ...同じ動作をします。より良い方法は、1つのメソッドAddFunctionalityForPressを作成し、異なる値をパラメータとして配置することです。
  5. AddButtonFunctionality、AddFunctionalityForShortPressこれらのメソッドを拡張メソッドとして実行します。 buttonFunctionality.AddButtonFunctionality(short).AddButtonFunctionality(long)...のようなロジックシーケンスを見ることができます。
関連する問題