如何重构看起来相似的方法调用？

Question

如何重构看起来相似的方法调用？

3

我有不同的水果类，它们都实现了同一个接口IFruit：

public interface IApple : IFruit{ }  
public interface IBanana : IFruit{ }  
public interface ICarrot: IFruit{ }

每个人都有自己的抽屉:

public class AppleDrawer
{
    public void Draw(IApple apple, Graphics graphics){}
}

public class BananaDrawer
{
    public void Draw(IBanana banana, Graphics graphics){}
}

如果我想绘制一个水果列表，我需要执行以下操作：

public void DrawFruits(List<IFruit> fruits, Graphics graphics)
{
    foreach(var fruit in fruits)
    {
        if(fruit is IBanana)
        {
            var banana = (IBanana)fruit;
            var drawer = new BananaDrawer();
            drawer.Draw(banana, graphics);
        }
        else if(fruit is IApple)
        {
            var apple = (IApple)fruit;
            var drawer = new AppleDrawer();
            drawer.Draw(banana, graphics);
        }
        etc...

}

当我阅读我的代码时，我感到非常糟糕。
我的问题在于多个if..else语句，因为我有12种不同的水果，在当前项目中需要经常使用这个语句。

有没有一种重构我的DrawFruits方法的方法？
我正在考虑一种工厂模式，但我不太清楚如何实现。
我的水果类必须将Drawer作为属性吗？或者我可以调用一个Drawer工厂方法？

这是我在当前项目中经常遇到的一种模式，我找不到令我满意的解决方案。

- Cyril Gandon

2

为什么不直接使用 IFruitDrawer？ - Cody Gray

4个回答

4

也许你可以创建一个抽象类 Fruit，其中包含Draw方法，再相应地创建一个抽象类 FruitDrawer。

例如：


public abstract class Fruit {
  ...
}

public abstract class FruitDrawer {
  public void Draw(Fruit f, Graphics g)
  {
    ...
  }
}

- Joel A. Villarreal Bertoldi

FruitDrawer.Draw 将具有任何要绘制的水果的共同元素。您可以使用 @parapura rajkumar 提出的模式。 - Joel A. Villarreal Bertoldi

这样一来，您将在FruitDrawer类中再次拥有if-then-else，没有任何变化。 - Saeed Amiri

2

您基本上重复了一个经典的开闭原则违反例子，只不过将形状换成了水果。

如果您学习SOLID原则，您就能学到更多知识，虽然可以深入探讨您的问题，但是根据请求，我会进入您的代码。

首先，我会将Draw推到IFruit接口中，以便每个水果负责绘制自己，而不是控制器类操纵一切。您可能会遇到单一职责（SOLID中的S）违反，但这将是另一个重构。重要的是这里的水果自己画，并且控制器为扩展（添加更多水果类）而打开，但对于修改却是关闭的（因为它们自己画，控制器从不改变）。

然后你就有了一个简单的循环来绘制你的水果。

foreach(var fruit in fruits)
    fruit.Draw(...);

为了回应“如何不破坏它们”的评论...

了解它们是学习和应用它们的职业生涯的第一步。避免破坏它们的最简单方法（并不完全简单）是非常严格地执行测试驱动开发。你所举的大部分例子都很难通过TDD有意识地完成。换句话说，如果你知道SOLID原则并且实施TDD，你就不会得到你发布的代码。

- Austin Salonen

2

我并不是提问者，但即使是我，我仍然希望您至少深入了解一些问题。是的，通过阅读和学习课程，你会学到更多知识，但如果每个人都这样做，这个网站就没有太大意义了。请考虑至少总结你引用的设计原则以及可能的“修复”（改进设计）。 - Cody Gray

真的，因为我读了SOLID原则，正是因为我读了它们，所以我知道我违反了它们。但我仍然不知道如何遵循它们！ - Cyril Gandon

根据您的要求进行编辑。 - Austin Salonen

@Scorpi0：根据您的评论进行了编辑。 - Austin Salonen

1

我认为这样做会更好：

interface IFruit
{
   void Draw();
}

class Banana : IFruit
{
   void Draw()
   {
      BananaDrawer drawer = new BananaDrawer();
      drawer.Draw();
   }
}

并且简单地使用它：

foreach(var fruit in fruits)
   fruit.Draw();

实际上，通过这种方式，您可以在相关水果的特定抽屉中运行额外的初始化。

- Saeed Amiri

我已经考虑过这个问题了，我只需要创建一个部分类，因为水果和抽屉在不同的层上。这是最好的方法吗？ - Cyril Gandon

@Scorpi0，这是我想到的，我不知道它是否是最好的，但与其他当前可用的答案相比，我认为这是最好的。实际上，您可以仅使用接口而不是具体类将它们分开，我没有这样做只是为了缩写主要部分。 - Saeed Amiri

网页内容由stack overflow 提供, 点击上面的

可以查看英文原文，
原文链接

- parapura rajkumar · Accepted Answer

一种方法是在您的IFruit上拥有一个GetDrawer。

public interface IFruit 
{
    BaseDrawer GetDrawer();
}

还有一个BaseDrawer接口

public interface BaseDrawer
{
    void Draw(IFruit fruit, Graphics graphics);
}.

public class AppleDrawer : BaseDrawer
{
    public void Draw(IFruit apple, Graphics graphics) { }
}

public class BananaDrawer : BaseDrawer
{
    public void Draw(IFruit banana, Graphics graphics) { }
}

现在，您的绘画水果变得非常简单。

   public void DrawFruits(List<IFruit> fruits, Graphics graphics)
    {
        foreach (var fruit in fruits)
        {
            var drawer = fruit.GetDrawer();
            drawer.Draw(fruit, graphics);
        }
    }

有时候你需要一个抽屉（Drawer）、绘图仪（Plotter）和打印机（Printer），这样你的水果手机（IFruit）可能会变得太重，就像下面这样。

public interface IFruit 
{
    BaseDrawer GetDrawer();
    BasePrinter GetPrinter();
    BasePlotter GetPlotter();
}

访问者模式是一个很好的解决方案。基本上你会有：

 public interface iFruit
   {
      void Accept(FruitVisitor visitor);
   }

所有可能的绘图访问只需要一个类。

public class DrawVisitor : FruitVisitor 
   {
      public override void Visit(Apple apple)
      {
         //draw the apple
      }

      public override void Visit(Banana banana)
      { 
         // draw the banana
      }
   }

这里你只需要一个 DrawVisitor 而不是 AppleDrawer，BananaDrawer等，所有的绘制代码都整齐地放在一个地方。你可能最终需要 PlotterVisitor、PrinterVisitor等。