28

我有一个用于将我的应用程序资源更新到当前应用程序版本的代码。此代码在应用程序更新后调用。

int version = 1002;   // current app version

switch(version)
{
   case 1001:
      updateTo1002();
      goto case 1002;

   case 1002:
      updateTo1003();
      goto case 1003;

   case 1003:
      updateTo1004();
      goto case 1004;
      break;

   case 1004:
      updateTo1005();
      break;
}

这里我们有一个级联方法通过跳转到指定的案例块来调用。我想知道 - 在这种情况下使用 go to 是一种好的做法(通常被认为是这样的坏做法!)?我不想一个接一个地调用方法——像这样:

updateTo1002()
{
   // do the job
   updateTo1003();
}
updateTo1003()
{
   // do the job
   updateTo1004();
}

是否有任何设计模式描述了这样的问题?

4

13 回答 13

58

好吧,如果我们想成为“面向对象”,为什么不让对象说话呢?

var updates = availableUpdates.Where(u => u.version > ver).OrderBy(u => u.version);
foreach (var update in updates) {
  update.apply();
}
于 2010-10-27T19:50:02.583 回答
40

在示例中,版本正在增加,并且总是按顺序调用较早的版本。我认为if这里的一组陈述可能更合适

if (version == 1001 ) { 
  updateTo1002();
}

if (version <= 1002) {
  updateTo1003();
}

if (version <= 1003) {
  updateTo1004(); 
}

if (version <= 1004) {
  updateTo1005();
}

一些人评论说,随着版本数量的增加(想想 50 左右),这种方法是不可维护的。在这种情况下,这是一个更易于维护的版本

private List<Tuple<int, Action>> m_upgradeList;

public Program()
{
    m_upgradeList = new List<Tuple<int, Action>> {
        Tuple.Create(1001, new Action(updateTo1002)),
        Tuple.Create(1002, new Action(updateTo1003)),
        Tuple.Create(1003, new Action(updateTo1004)),
        Tuple.Create(1004, new Action(updateTo1005)),
    };
}

public void Upgrade(int version)
{
    foreach (var tuple in m_upgradeList)
    {
        if (version <= tuple.Item1)
        {
            tuple.Item2();
        }
    }
}
于 2010-10-27T19:40:26.727 回答
6

我讨厌不提供支持信息的空白语句,但 goto 相当普遍(有充分理由)并且有更好的方法来实现相同的结果。您可以尝试责任链模式,该模式将获得相同的结果,而无需 goto 实现可能变成的“意大利面条式”粘性。

责任链模式。

于 2010-10-27T19:43:05.687 回答
5

goto总是被认为是不好的做法。如果您使用 goto,通常更难阅读代码,并且您总是可以以不同的方式编写代码。

例如,您可以使用链表创建一个方法链和一些处理该链的处理器类。(请参阅 pst 的答案以获取很好的示例。)。它更加面向对象和可维护。或者,如果您必须在 beetween1003和 case之间再添加一个方法调用1004怎么办?

当然看到这个问题。

替代文字

于 2010-10-27T19:37:46.910 回答
2

我会建议命令模式的变体,每个命令都是自我验证的:

interface IUpgradeCommand<TApp>()
{
    bool UpgradeApplies(TApp app);
    void ApplyUpgrade(TApp app);
}

class UpgradeTo1002 : IUpgradeCommand<App>
{
    bool UpgradeApplies(App app) { return app.Version < 1002; }

    void ApplyUpgrade(App app) {
        // ...
        app.Version = 1002;
    }
}

class App
{
    public int Version { get; set; }

    IUpgradeCommand<App>[] upgrades = new[] {
        new UpgradeTo1001(),
        new UpgradeTo1002(),
        new UpgradeTo1003(),
    }

    void Upgrade()
    {
        foreach(var u in upgrades)
            if(u.UpgradeApplies(this))
                u.ApplyUpgrade(this);
    }
}
于 2010-10-27T20:11:48.750 回答
2

为什么不:

int version = 1001;

upgrade(int from_version){
  switch (from_version){
    case 1000:
      upgrade_1000();
      break;
    case 1001:
      upgrade_1001();
      break;
    .
    .
    .
    .
    case 4232:
      upgrade_4232();
      break;
  }
  version++;
  upgrade(version);
 }

当然,所有这些递归都会产生开销,但并不是那么多(调用 carbage 收集器时只有一个上下文和一个 int),而且所有这些都打包好了。

请注意,我不介意 goto 在这里,元组 (int:action) 变体也有其优点。

编辑:

对于那些不喜欢递归的人:

int version = 1001;
int LAST_VERSION = 4233;

While (version < LAST_VERSION){
  upgrade(version);
  version++;
}

upgrade(int from_version){
  switch (from_version){
    case 1000:
      upgrade_1000();
      break;
    case 1001:
      upgrade_1001();
      break;
    .
    .
    .
    .
    case 4232:
      upgrade_4232();
      break;
  }

}
于 2010-10-28T18:09:28.040 回答
1

我想说这是使用 GOTO 功能的一个非常合适的理由。

http://weblogs.asp.net/stevewellens/archive/2009/06/01/why-goto-still-exists-in-c.aspx

事实上,switch()C# 中的语句实际上是一组标签和隐藏的 goto 操作的漂亮面孔。case 'Foo':只是在 a 范围内定义标签类型的另一种方式switch()

于 2010-10-27T19:38:13.140 回答
1

我认为这里的逻辑可能有些倒退并导致了问题。如果您的方法如下所示:

updateTo1002() 
{ 
   if (version != 1001) {
       updateTo1001();
   }
   // do the job     
} 
updateTo1003() 
{ 
   if (version != 1002) {
       updateTo1002();
   }
   // do the job     
} 

我不知道您的确切用例,但在我看来,您通常希望更新到最新版本,但在此过程中根据需要安装增量更新。我认为这样做可以更好地捕捉到这种逻辑。

编辑:来自@user470379 的评论

在这种情况下,主要是识别您有复制/粘贴模式并对其进行编辑的事实。

在这种情况下,耦合问题几乎不是问题,但可能是。我会给你一些在你的场景中可能出现的东西,如果这样做的话,这些东西很难编码:

  • 现在每次更新都需要一个额外的清理步骤,所以在 updateTo1001() 之后调用 cleanup() 等。
  • 您需要能够退后一步才能测试旧版本
  • 您需要在 1001 和 1002 之间插入更新

让我们按照您的模式将这两者结合起来。首先让我们添加一个“undoUpgradeXXXX()”来撤消每次升级并能够后退。现在您需要第二组并行的 if 语句来执行撤消操作。

现在,让我们添加“插入 1002.5”。突然之间,您正在重写两条可能很长的 if 语句链。

您将遇到此类问题的关键迹象是您正在以某种模式进行编码。寻找这样的模式——事实上,我的第一个迹象通常是当我从别人的肩膀上看他们的代码时,如果我能看到一个模式,甚至无法阅读这样写的任何东西:

********
   ***
   *****

********
   ***
   *****
...

然后我知道他们的代码会有问题。

最简单的解决方案通常是从每个“组”中删除差异并将它们放入数据中(通常是数组,不一定是外部文件),将组折叠成一个循环并遍历该数组。

在您的情况下,简单的解决方案是使用单一升级方法制作每个升级对象。创建这些对象的数组,并在需要升级时对其进行迭代。您可能还需要某种方式来对它们进行排序——您当前正在使用一个数字,这可能会起作用——或者日期可能会更好——这样你就可以轻松地“转到”给定的日期。

现在有几点不同:

  • 向每次迭代(cleanup())添加新行为将是对循环的单行修改。
  • 重新排序将本地化为修改您的对象 - 可能更简单。
  • 将升级分成多个必须按顺序调用的步骤会很容易。

让我给你一个最后一个例子。假设在所有升级都运行之后,您需要为每个升级执行一个初始化步骤(在每种情况下都不同)。如果为每个对象添加一个初始化方法,那么对初始循环的修改是微不足道的(只需在循环中添加第二次迭代)。在您的原始设计中,您必须复制、粘贴和编辑整个 if 链。

结合 JUST undo & initialize,你有 4 个 if 链。最好在开始之前找出问题。

我也可以说消除这样的代码可能很困难(根据您的语言,这非常困难)。在 Ruby 中它实际上很容易,在 java 中可能需要一些练习,而且许多人似乎无法做到,所以他们称 Java 不灵活且困难。

花一个小时在这里和那里思考如何减少这样的代码对我的编程能力的影响比我读过的任何书或我接受过的任何培训都要多。

这也是一个挑战,让你有事可做,而不是编辑巨大的 if 链来寻找你忘记将 8898 更改为 8899 的复制/粘贴错误。老实说,它使编程变得有趣(这就是为什么我花了这么多时间在这个答案上)

于 2010-10-27T19:55:13.887 回答
1

正确的方法是使用继承和多态,如下所示:

首先,请注意在各种情况下执行的代码之间存在清晰的层次关系。IE。第一种情况为第二种情况做了一切,然后再做更多。第二种情况为第三种情况做了一切,然后更多。

因此,创建一个类层次结构:

// Java used as a preference; translatable to C#
class Version {
    void update () {
        // do nothing
    }
}

class Version1001 extends Version {
    @Override void update () {
        super.update();
        // code from case update 1001
    }
}

class Version1002 extends Version1001 {
    @Override void update () {
        super.update();
        // code from case update 1002
    }
}

class Version1003 extends Version1002 {
    @Override void update () {
        super.update();
        // code from case update 1003
    }
}

// and so forth

其次,使用虚拟调度,也就是多态,而不是 switch-case:

Version version = new Version1005();
version.update();

讨论(对于不相信的人):

  1. 代替 goto,使用与目的地无关的 super.update() 并在类层次结构“Version1002 extends Version1001”中建立连接
  2. 这不依赖于版本号之间的算术关系(与上面的流行答案不同),因此您可以优雅地执行“VersionHelios extends VersionGalileo”之类的操作
  3. 此类可以集中任何其他特定于版本的功能,例如@Override String getVersionName () { return "v1003"; }
于 2011-10-28T05:18:41.260 回答
0

我认为没关系,尽管我怀疑这是您的真实代码。您确定不需要将Update方法封装在AppUpdate类或其他东西中吗?XXX001IMO,您拥有名为等方法的事实XXX002并不是一个好兆头。

反正。这是代表的替代方案(并不是真的建议您使用它,只是一个想法):

var updates = new Action[] 
                 {updateTo1002, updateTo1003, updateTo1004, updateTo1005};

if(version < 1001 || version > 1004)
   throw new InvalidOperationException("...");    

foreach(var update in updates.Skip(version - 1001))
   update();

如果没有更多细节,很难推荐最合适的模式。

于 2010-10-27T19:47:39.713 回答
0

我不得不处理一些这样的问题(将文件转换成这种格式,所以它可以转换成另一种格式,等等)而且我不喜欢 switch 语句。带有“if”测试的版本可能很好,或者递归地有类似的东西可能很好:

/* 如果可能,至少升级到版本 106。如果代码
   无法进行升级,别管它,让外部代码
   观察不变的版本号 */

无效升级到 106(无效)
{
  如果(版本 < 105)
    upgrade_to_105();
  如果(版本 == 105)
  {
    ...
    版本 = 106;
  }
}

除非您有数千个版本,否则堆栈深度应该不是问题。我认为 if-test 版本,专门针对每个升级例程可以处理的版本进行测试,读起来更好;如果最后的版本号不是主代码可以处理的版本号,则发出错误信号。或者,在主代码中丢失“if”语句并将它们包含在例程中。我不喜欢“case”语句,因为它没有考虑版本更新例程可能不起作用或一次更新多个级别的可能性。

于 2010-10-27T20:48:18.873 回答
0

我发表了一条评论说,使用 goto 永远不值得你为使用它而付出的废话(即使它是一个很棒的、完美的使用)——太多的程序员学到了一些东西,却永远无法从他们的大脑中解脱出来。

我不打算发布答案,但我认为你暗示的整个解决方案都是错误的还不够清楚。我以为这只是为了说明您的观点,但应该明确指出:要非常小心代码中的模式——这与复制/粘贴代码一样糟糕(实际上,它是复制/粘贴代码)。

您应该只有一个对象集合,每个对象都有升级代码和版本号。

您只需在版本号 < 您的目标版本时迭代该集合,并为每个对象调用该对象的升级代码。通过这种方式创建一个新的升级级别,您只需制作一个“升级”对象并将其粘贴到集合中。

您的升级对象链甚至可以通过添加撤消和向控制器添加非常微不足道的代码来向后和向前遍历——这将成为使用示例代码维护的噩梦。

于 2010-10-27T21:52:35.313 回答
0

您可以查看状态机工作流模式。对你来说简单实用的可能是:Stateless project

于 2010-10-28T10:35:37.583 回答