2016-11-22 4 views
0

私は、同様の署名(ChooseChamferとChooseFillet)を持つ2つのメソッドを持っています。これらのメソッドは、if ... elseという拡張構造を持っています。 (私は単純化されたバージョンを持っていました)メソッドが呼び出されます(CreateChamferとCreateFillet)。どのようにして単一のメソッドでリファクタリングコードを作成できますか?2つのメソッドをどのようにリファクタリングするのですか?

private void ChooseChamfer(string featureType, double diameter, double distance, string str1, string str2) 
{ 
    if (featureType.Contains("F")) 
    { 
     CreateChamfer(diameter, 
      distance, 
      -double.Parse(str1, System.Globalization.CultureInfo.InvariantCulture), 
      double.Parse(str2, System.Globalization.CultureInfo.InvariantCulture)); 
    } 
    else if (featureType.Contains("L")) 
    { 
     CreateChamfer(diameter, 
      distance, 
      double.Parse(str1, System.Globalization.CultureInfo.InvariantCulture), 
      -double.Parse(str2, System.Globalization.CultureInfo.InvariantCulture)); 
    } 
} 

private void ChooseFillet(string featureType, double diameter, double distance, string str1) 
{ 
    if (featureType.Contains("F")) 
    { 
     CreateFillet(diameter, 
      distance, 
      -double.Parse(str1, System.Globalization.CultureInfo.InvariantCulture)); 
    } 
    else if (featureType.Contains("L")) 
    { 
     CreateFillet(diameter, 
      distance, 
      double.Parse(str1, System.Globalization.CultureInfo.InvariantCulture)); 
    } 
} 

private void CreateChamfer(double diameter, double distance, double str1, double str2) 
{ 
    //Draw Chamfer 
} 

private void CreateFillet(double diameter, double distance, double str1) 
{ 
    //Draw Fillet 
} 

ありがとうございます。

+1

この種の質問には[http://codereview.stackexchange.com/](http://codereview.stackexchange.com/)が良い場所だと思います。 – Fabio

+0

あなたは反射( 'MethodInfo')で作業することができましたが実行時の例外が発生する可能性があります。または、intを逆のメソッドに分割します。 – NtFreX

+0

'Chamfer'と' Fillet'は基本クラスを共有していますか?そうすれば、制約のあるジェネリックスは物事を少し単純化するかもしれません。また、 'featureType'のif-elseチェックの大集合を持っていれば、呼び出されるメソッドの値を' Dictionary'にリファクタリングすることができます。 – KMoussa

答えて

0

私はあなたがどのくらい深く切っているのか分かりませんが、それが複雑な場合はいつでも工場と専門家と思っています。これは、100種類の方法をスライスすることができますが、私は潜在的な複雑さから理解して何から、私はFeatureTypeに工場を基づかことは面取り/フィレットの上にそれを基づかよりも良いことだと思う:

abstract class GraphicAttributes 
{ 
    public double Diameter { get; protected set; } 
    public double Distance { get; protected set; } 
    public double Str1 { get; protected set; } 
    public double? Str2 { get; protected set; } 

    public GraphicAttributes(double diameter, double distance, string str1, string str2) 
    { 
     // load all attributes plain-vanilla - let the subclasses adjust as necessary 
     Diameter = diameter; 
     Distance = distance; 
     Str1 = double.Parse(str1, System.Globalization.CultureInfo.InvariantCulture); 
     Str2 = String.IsNullOrEmpty(str2) 
      ? new Nullable<double>() 
      : double.Parse(str2, System.Globalization.CultureInfo.InvariantCulture); 
    } 
} 

class FeatureTypeF : GraphicAttributes 
{ 
    public FeatureTypeF(double diameter, double distance, string str1, string str2) 
     : base(diameter, distance, str1, str2) 
    { 
     Str1 = -Str1; 
    } 
} 

class FeatureTypeL : GraphicAttributes 
{ 
    public FeatureTypeL(double diameter, double distance, string str1, string str2) 
     : base(diameter, distance, str1, str2) 
    { 
     if (Str2.HasValue) 
      Str2 = new Nullable<double>(-(Str2.Value)); 
    } 
} 

class GraphicAttributeFactory 
{ 
    public static GraphicAttributes GetGraphicAttributes(string featureType, double diameter, double distance, string str1, string str2) 
    { 
     if (featureType.Contains("F")) 
      return new FeatureTypeF(diameter, distance, str1, str2); 

     if (featureType.Contains("L")) 
      return new FeatureTypeL(diameter, distance, str1, str2); 

     // add more implementations here 

     throw new Exception("Unexpected featureType!"); 
    } 
} 

// and then your final method looks like: 

private void Choose(string featureType, double diameter, double distance, string str1, string str2) 
{ 
    GraphicAttributes ga = GraphicAttributeFactory.GetGraphicAttributes(featureType, diameter, distance, str1, str2); 

    if (ga.Str2.HasValue) 
     CreateChamfer(ga.Diameter, ga.Distance, ga.Str1, ga.Str2.Value); 
    else 
     CreateFillet(ga.Diameter, ga.Distance, ga.Str1); 
} 

呼び出し側が明示的に無効にする必要があります余分なパラメータ:

Choose("__L_", 12.0, 5.0, "123", "456"); // chamfer 
Choose("__F_", 12.0, 5.0, "123", null); // fillet 

その部分は不器用です。 'params string'を使用するように拡張することができます。

また、GraphicAttributeインスタンスを直接取得するCreate ..()メソッドをより深くリファクタリングすることができれば幸いです。

実際の考え方は、属性の特定の扱いを専門クラスに入れて、それぞれ独自の状況を処理する方法を知っているし、適切な実装を選択する方法を知っているファクトリです。

関連する問題