代码重构功课？

Question

这是我必须为我的作业重构的代码:

if (state == TEXAS) {
    rate = TX_RATE;
    amt = base * TX_RATE;
    calc = 2 * basis(amt) + extra(amt) * 1.05;
} else if ((state == OHIO) || (state == MAINE)) {
    rate = (state == OHIO) ? OH_RATE : MN_RATE;
    amt = base * rate;
    calc = 2 * basis(amt) + extra(amt) * 1.05;
    if (state == OHIO)
        points = 2;
} else {
    rate = 1;
    amt = base;
    calc = 2 * basis(amt) + extra(amt) * 1.05;
}

我做过这样的事情

if (state == TEXAS) {
    rate = TX_RATE;
    calculation(rate);
} 
else if ((state == OHIO) || (state == MAINE))
    {
rate = (state == OHIO) ? OH_RATE : MN_RATE;

calculation(rate);

if (state == OHIO)
    points = 2;
}

else {
    rate = 1;
    calculation(rate);
}

function calculation(rate)
{
    amt = base * rate;
    calc = 2 * basis(amt) + extra(amt) * 1.05;
}

我怎么能做得更好？
编辑我已完成代码编辑 amt = base * rate;

Answer 1

class State {
private :
  double taxRate;
  int baseWeight;
  int extraWeight;
  string name;
  base;
public:
  State(string name, double taxRate = 1, int point =0, double baseWeight=2, double extraWeight=1.05); //implement the method yourself
  double extra(double base);
  double basis(double base);
  double calculate(double base){
      return baseWeight * basis(base) + baseWeight * extra(base);
  }
  int point(){return point};

};

现在该如何使用它:

State ohio ("OHIO", OH_RATE, 2);
cout << "OHIO result:" ohio.calculate() << " point:" << ohio.point() << endl;

Answer 2

有人想过为这个做一个真正的OO解决方案吗？如果我在一个声称是OO的项目中遇到过这样的代码,我会认真地说它不是.

如果您看到代表不同类型对象的变量(如"state"),那么您可以很好地进行继承.虽然没有给它太多(因为它毕竟是你的功课),你可能想要沿着这些方向做一些事情(伪代码 - 希望你明白这个想法).

 abstract class State:
   protected abstract int getAmt()

   protected int basis(amt):
      return ...?

   protected int extra(amt):
      return ...?

   public int getPoints()
      return 1 // Just a guess ?

   public final int calculateTax():
      return 2 * basis(getAmt()) + extra(getAmt()) * 1.05


 final class DefaultState > State:
   protected int getAmt():
      return base


 final class Texas > State:
   protected int getAmt():
      return base * TX_RATE


 final class Ohio > State:
   public getPoints():
      return 2

   protected int getAmt():
      return base * OH_RATE


 final class Ohio > State:
   protected int getAmt():
      return base * MN_RATE

这里使用的概念称为"Open Recursion",以防您想知道

Answer 3

史蒂夫关于这个switch陈述的观点很好,但我想提出一个不同的方法:数组.

如果将速率信息存储在数组中并按状态索引,则可以更长时间地维护此类代码.

考虑一下:

#define OTHER 0
#define OHIO 1
#define MAINE 2
#define TEXAS 3

int rates[4];
rates[OTHER] = ...
rates[OHIO] = ...
rates[MAINE] = ...
rates[TEXAS] = ...

看看这可能会让一个calculate函数以不同的方式运行.(注意,在"现实生活中",int rates[4]数组可以通过许多不同的方式完成- 一个hashmap,一个简单的struct rate { char state[12]; int rate; }对象数组,其状态名称和速率在运行时一起存储,或者是一个简单的静态分配数组int rates[4] = {0, 2, 3, 10};.我选择了这个因为它显示按#defined内容索引数组.enum也有效.)

Answer 4

你有一个'Java'标签,所以假设这实际上是Java友好的,我会用Enum来做:

enum USStates
{
    TEXAS(TX_RATE), OHIO(OH_RATE), MAINE(MN_RATE), OTHER(1);

    final double rate;

    USStates(double rate)
    {
        this.rate = rate;
    }

    public double calc(double base)
    {
        double amt = amt(base);
        return 2.0 * basis(amt) + extra(amt) * 1.05;
    }

    public double amt(double base)
    {
        return base * rate;
    }
}

然后,在您的实际可执行代码中:

    rate = state.rate;
    amt = state.amt(base);
    calc = state.calc(base);

    if (USStates.OHIO == state)
    {
        points = 2;
    }

如果"base"是一个常量(从示例代码中不清楚),可以通过直接访问它作为final来进一步简化,而不是将其作为参数传递.

该解决方案有一些好处.首先,状态的实际速率实际上不需要使用命名约定在它们自己的单独常量中,但实际上可以作为Enum本身的一部分存储(因此实际上只需输入"TEXAS(TX_RATE)"而不是"TEXAS(1.4)"(或其任何值))然后将该费率作为"TEXAS"枚举类型的一部分进行维护.

它还具有以下优点:计算逻辑与其操作的常量一起被捕获(封装,甚至).

通过使用Enums,您可以确保人们不会意外地对常量使用无效操作(例如意外地对它们执行数学运算).

通过减少条件语句的数量,可以显着减少可能的执行路径数.较少可能的路径意味着空指针和未初始化变量的可能性较少.(如示例代码所示,对于OHIO以外的任何状态,"点"存在未初始化的变量错误的风险)