且听书吟

  • 首页
  • 分类
  • 关于
  • 留言
  • 友链
扫码加我微信好友

微信

且听书吟

在编程中思考,简化你的判断逻辑

2017-03-06 20:31 思考 编码 编程

在编程中思考,简化你的判断逻辑

之前看 Linus Toward 在去年的某次采访中说到的好代码坏代码,当中提到了逻辑的精简,能用更通用的逻辑减少 if else 的判断在某种程度上可以使你的代码变得更好。最近一段时间重构了部分老代码,也 Review 了不少代码,对此观点深有感触。

很多时候,程序员接到的需求,产品巴不得你立刻就能搞定,有时候会给非常紧迫的时间点。这种情况下会带来的最直接问题,就是“设计坏味”。有整体的架构设计的不合理,也有代码逻辑的问题。尤其是对于边界条件的处理,因为需求的急,很多时候大家就会按照业务语言去写。

比如,下面这个例子:

某一报警系统产生了报警邮件,现在需要按照类型显示不同的内容。

类型报警选择对象邮件中展示内容
Web事务单条Web事务Tier,Web事务,节点
Web事务TierTier,节点
节点某一个节点节点
节点TierTier,节点

如果按照业务的语言,我们可能写出如下的伪代码:

if (Web事务&& 单条Web事务) {
    return Tier,Web事务,节点;
}
if (Web事务&& Tier) {
    return Tier,节点;
}
if (节点&& 某一个节点) {
    return 节点;
}
if (节点&& Tier) {
    return Tier,节点;
}

可能你看到这里会立刻笑出来,哪有只写 if 不写 else 的。那好,也许你会写出这样的代码。

if (Web事务) {
    if (单条Web事务) {
        return Tier,Web事务,节点;
    } else if (Tier) {
        return Tier,节点;
    }
} else if (节点) {
    if (某一个节点) {
        return 节点;
    } else if (Tier) {
        return Tier,节点;
    }
}

我相信,大部分人都能将判断逻辑写到这一层,但是,这就完了么?也许你会说完了,逻辑也正确,看起来也很清晰。然而,这远远不够。

其实我们可以发现,在每种情况下,都会返回 节点 信息。只返回节点信息的只有一种情况,其他的情况下,基本都返回 Tier 信息。只有 Web事务 && 单条Web事务 的情况需要返回 Web事务 信息。所以,最后我们可以精简为两个判断。

result = "节点";
if (Web事务&& 单条Web事务) {
    result = "Web事务" + result;
}
if (Web事务 || !某一个节点) {
    result = "Tier" + result;
}
return result;

回到最初的命题,为何不要用业务的语言来编写判断逻辑呢?因为业务语言是给用户和产品看的,他们在描述上本身就不够精简。其次,业务的描述,很多时候,是定义边界,说明问题,而不是告诉你判断逻辑。

所以,在写代码的时候,更多的时候要细化逻辑。这样,在维护修改时,才更为方便。下面我举另一个更具体的例子。

这是我 Review Scala 代码的时候遇到的一个问题,首先我先用业务语言描述一下需求。需要判断某个规则的开闭状态,在一天的几点到几点间启用,且还可以额外设置是周一到周日的哪几天启用。

于是我看到当时的同事,写一个方法 isSuppressTime ,会给两个参数,第一个参数为一个时间戳 timestamp ,第二个参数为一个 List[Map[String, String]] 。

Map[String, String] 里面有4个值,分别是:

  1. startTime,从零点到某个具体开始时间的秒数,0~86399。
  2. endTime,从零点到某个具体结束时间的秒数,0~86399。
  3. isDaily,当时间戳在 startTime endTime 之间时,如果此项为 true,则返回 true。
  4. weekdays,周一到周日,1~7,以,间隔组成的字符串,如1,4,5。表示周一、周四、周五且时间戳在 startTime endTime 之间时返回 true

最后他写出了如下的代码(Scala):

def isSuppressTime(now: Long = System.currentTimeMillis(),
                   suppressTimes: java.util.List[java.util.Map[String, String]] = rule.getSuppressTime): Boolean = {
  if(suppressTimes == null)
    return false
  import scala.collection.JavaConversions._
  suppressTimes.foreach(params => {
    if (params != null && params.size > 0) {
      val startTime = params.get("startTime")
      val endTime = params.get("endTime")
      val isDaily = params.get("isDaily")
      val weekdays = params.get("weekday").split(",").toList

      val zero = zeroTimestamp()
      //今天零点零分零秒的毫秒数
      val start = zero + startTime.toLong * 1000
      val end = zero + endTime.toLong * 1000

      if (start < = now&& end >= now) {
        if (isDaily.toBoolean)
          return true
        val cal = Calendar.getInstance()
        cal.setTimeInMillis(now)
        var day = cal.get(Calendar.DAY_OF_WEEK)
        if (day == 1)
          day = 7
        else
          day = day - 1
        val weekday = day.toString
        if (weekdays.contains(weekday))
          return true
      }
    }
  })
  return false
}

private def zeroTimestamp(): Long = {
  val cal = Calendar.getInstance()
  cal.set(Calendar.HOUR_OF_DAY, 0)
  cal.set(Calendar.SECOND, 0)
  cal.set(Calendar.MINUTE, 0)
  cal.set(Calendar.MILLISECOND, 1)
  cal.getTimeInMillis()
}

这个代码看着很长,判断很多,而且还用了超级多陈旧的 API,使得它的性能也不好。而且这是一段 Scala 的代码,却用了较多的 return 和 var ,这两个都是 Scala 不提倡的。最关键的,明明是函数式的代码,他却写出了过程式的感觉。给后面的维护人员(我)带来了不少困扰。

下面,我们来一点点优化这段代码(需要一点点 Java 功底),首先对于方法 zeroTimestamp() ,我第一眼看到的时候,是惊讶的,原作者用了一个比较旧的 Calendar 。对它最深刻的印象就是当年写 SimpleDataFormat 的时候,因为 Calendar 这货导致线程不安全。而每次创建 SimpleDataFormat 的开销比较大,最后不得不写了个 ThreadLocal<simpledataformat> 。

而Java8以后,我们可以直接用 java.time 下面的类来改写 zeroTimestamp() ,这里只需要一行代码,如下:

private def zeroTimestamp(): Long = {
  Timestamp.valueOf(LocalDate.now().atStartOfDay()).getTime
}

回到 isSuppressTime 方法上来,我姑且不说他的设计多么麻烦,因为这是一个已经被广泛使用的方法,我能做到的就是在不改变签名的情况写来优化实现。

首先,开头我们就看到

if(suppressTimes == null)
  return false

这个空的判断和强制 return ,在 Java 里面, null 是一个很痛苦的事情,Scala 因为基于 JVM 也不例外,但是 Scala 和 Java 8 都分别有一个 Option 类(Java8是 Optional ),来做空值处理。

所以,我们这里第一时间可以干掉这个判断,写成如下方式

Option[util.List[util.Map[String, String]]](suppressTimes).map(times => {
  // ba la ba la
}).getOrElse(false)

代码里面的 times 为方法中非空的 suppressTimes ,注释部分为核心的处理逻辑,这样我们避免了一个 if 判断,减少了一个 return 。

而其实, Option([A]).map([B] => Boolean).getOrElse(false) 等价于 Option 的 exists 方法,最后完整的代码应该如下:

def isSuppressTime(now: Long = System.currentTimeMillis(),
        suppressTimes: java.util.List[java.util.Map[String, String]]): Boolean = {
  Option[util.List[util.Map[String, String]]](suppressTimes).exists(times => {
    times.foreach(params => {
      if (params != null&& params.size > 0) {
        val startTime = params.get("startTime")
        val endTime = params.get("endTime")
        val isDaily = params.get("isDaily")
        val weekdays = params.get("weekday").split(",").toList

        val zero = zeroTimestamp()
        //今天零点零分零秒的毫秒数
        val start = zero + startTime.toLong * 1000
        val end = zero + endTime.toLong * 1000

        if (start < = now&& end >= now) {
          if (isDaily.toBoolean)
            return true
          val cal = Calendar.getInstance()
          cal.setTimeInMillis(now)
          var day = cal.get(Calendar.DAY_OF_WEEK)
          if (day == 1)
            day = 7
          else
            day = day - 1
          val weekday = day.toString
          if (weekdays.contains(weekday))
            return true
        }
      }
    })
    false
  })
}

在上面的例子里面,我们已经干掉了两个 return 和一个 if ,让它有一点 Functional 的感觉了。

上面重构后代码中, times 的类型是 util.List[util.Map[String, String]] 。 times.foreach(params => {}) 里的 params 对应的是 util.Map[String, String] 类型。所以,我们看到判断 if (params != null && params.size > 0) 时应该会发现,这就是一个list filter的过程嘛。 .filter() 之后不就是一个 .map() ,于是我们可以这么改:

Option[util.List[util.Map[String, String]]](suppressTimes).exists(times => {
  times.filter(params => params != null&& !params.isEmpty).map(params => {
    // ba la ba la
  }).contains(true)
})

当然 .map(xxx => Boolean).contains(true) 等价于 .exists() ,于是我们重构的方法如下:

def isSuppressTime(now: Long = System.currentTimeMillis(),
        suppressTimes: java.util.List[java.util.Map[String, String]]): Boolean = {
  Option[util.List[util.Map[String, String]]](suppressTimes).exists(times => {
    times.filter(params => params != null&& !params.isEmpty).exists(params => {
      val startTime = params.get("startTime")
      val endTime = params.get("endTime")
      val isDaily = params.get("isDaily")
      val weekdays = params.get("weekday").split(",").toList

      val zero = zeroTimestamp()
      //今天零点零分零秒的毫秒数
      val start = zero + startTime.toLong * 1000
      val end = zero + endTime.toLong * 1000

      if (start < = now&& end >= now) {
        if (isDaily.toBoolean)
          return true
        val cal = Calendar.getInstance()
        cal.setTimeInMillis(now)
        var day = cal.get(Calendar.DAY_OF_WEEK)
        if (day == 1)
          day = 7
        else
          day = day - 1
        val weekday = day.toString
        if (weekdays.contains(weekday))
          return true
      }
      false
    })
  })
}

这次重构,我们又去掉了一层 if 判断,改为 filter 实现,减少了一次返回。到了这一步,我们会发现,下面需要实现的就是一个方法,对 util.Map[String, String] 去做处理,返回一个布尔值,于是我们精简方法的实现代码为两部分:

def isSuppressTime(now: Long = System.currentTimeMillis(),
                   suppressTimes: java.util.List[java.util.Map[String, String]] = rule.getSuppressTime): Boolean = {
  Option[util.List[util.Map[String, String]]](suppressTimes)
    .exists(times => times.filter(params => params != null&& !params.isEmpty).exists(isValidateTimes(_, now)))
}

这个为边界过滤的方法。

private def isValidateTimes(params: java.util.Map[String, String], now: Long): Boolean = {
  val startTime = params.get("startTime")
  val endTime = params.get("endTime")
  val isDaily = params.get("isDaily")
  val weekdays = params.get("weekday").split(",").toList

  val zero = zeroTimestamp()
  //今天零点零分零秒的毫秒数
  val start = zero + startTime.toLong * 1000
  val end = zero + endTime.toLong * 1000

  if (start < = now&& end >= now) {
    if (isDaily.toBoolean)
      return true
    val cal = Calendar.getInstance()
    cal.setTimeInMillis(now)
    var day = cal.get(Calendar.DAY_OF_WEEK)
    if (day == 1)
      day = 7
    else
      day = day - 1
    val weekday = day.toString
    if (weekdays.contains(weekday))
      return true
  }
  false
}

这个为我们要重构的核心处理逻辑。我们优化整理它的判断条件,最后可以实现如下的完整代码:

def isSuppressTime(now: Long = System.currentTimeMillis(),
                   suppressTimes: java.util.List[java.util.Map[String, String]] = rule.getSuppressTime): Boolean = {
  Option[util.List[util.Map[String, String]]](suppressTimes)
    .exists(times => times.filter(params => params != null&& !params.isEmpty)
    .exists(isValidateTimes(_, now)))
}

private def isValidateTimes(params: java.util.Map[String, String], now: Long): Boolean = {
  val zero = zeroTimestamp()
  val start = zero + params.get("startTime").toLong * 1000
  val end = zero + params.get("endTime").toLong * 1000
  val isDaily = params.get("isDaily").toBoolean
  val weekdays = params.get("weekday").split(",").toList
  val weekday = LocalDate.now().getDayOfWeek.getValue.toString

  (start < = now&& end >= now)&& (isDaily || weekdays.contains(weekday))
}

这样,我们就实现了一个 if 都没有,一个 return 都没有的纯函数式写法。

总的来说,代码谁都能写出来,但是把需求从文字或者是流程描述换成编码实现时就有了对程序员抽象逻辑能力的需求。如何组织抽象,就像是如何写作文,或者是 Kata(空手道里面的招数、套路),不要按照业务描述写 if else,而要尽可能简化找到一致性的简单逻辑描述。

如果能将所有的特殊情况变为通用情况,简化逻辑判断,那么代码在后面的迭代中也会比较易于维护。

微信扫一扫 分享朋友圈

在微信中请长按二维码

评论 (6)
头像
  • cross
    cross

    第三次,重复时候重构它。。。

    2025-01-18 08:41
  • ScienJus
    ScienJus

    第一次见到能把 Scala 写的这么惨的代码…

    2017-05-06 08:37
    • 雨帆
      雨帆 站长

      你不知道我看那块代码的时候有多痛苦,此类代码还不少。

      2017-05-06 08:45
  • 掩耳
    掩耳

    文科生,不懂代码

    2017-03-26 03:47
  • 故事会
    故事会

    顶一下博主的观点。。。

    2017-03-21 00:15
  • themebetter
    themebetter

    抢个沙发

    2017-03-14 00:44
随机文章
  • 写在今天——记重开始写博
  • 关于合肥这个“依壁雕凿”的城市
  • 碗
  • 变与不变
  • 我的Linux学习历程:那些我看过的Linux书籍们
近期评论
  • 白熊阿丸发表在《蓝色时期》
  • 蒙需发表在《蓝色时期》
  • SUUS发表在《蓝色时期》
  • 龙一发表在《蓝色时期》
  • Moran发表在《友人帐》
文章标签
成长初中HTML奥运会青春期笨蛋凤凰花饮食恋恋思念
单向历
Copyright © 2011-2025 且听书吟
皖ICP备2021002315号-2 萌ICP备20200318号
Astro Badge