阅读 1284

一段代码的重构实践记录

这篇博客谈一下在实际项目中我们如何执行重构。

首先我们明确一下重构的目标是什么?重构是为了让项目中的代码易懂,易维护。我觉得有一些像家居中的收纳。假设你有一个抽屉,现在你只有一样东西。那么需要去整理收纳吗?其实意义不大,因为任何人只要打开抽屉,就能知道里面装了什么。但是随着业务需求的增长,抽屉里的东西越来越多,往里面放东西的人也越来越多。终于过了一个临界点,任何一个人要往抽屉里找东西都越来越难。

所以我们需要保持秩序。这是收纳,也是重构。

下面以我在重构自定义地图控件中项目里看到的一段代码为例,来说明一下重构如何执行。 首先介绍一下需求:在地图上我们要绘制一个多边形,多边形的顶点需要支持拖动,每次顶点被拖动后,多边形区域就需要重新绘制。为了让用户在编辑区域的时候更加友好,在编辑时我们还会展示每条边的边长。

下面的代码的作用就是绘制多边形。

class CustomMapOverlayView: UIView {
    var polygonEditPointViews = [PolygonAnnotationView]()
    var polygonLayer = CAShapeLayer()
    var distanceMarkerLayer = CAShapeLayer()

	private func drawPolygon(currentIndex: Int = 0, showDistanceMarker: Bool = true) {
        guard polygonControlPointViews.count >= 3 else { return }
        polygonEditPointViews.forEach { $0.removeFromSuperview() }
        polygonEditPointViews.removeAll()
        var distanceMarkerLayers = [CAShapeLayer]()
        let polygonPath = UIBezierPath()
        
        for (index, polygonControlPointView) in polygonControlPointViews.enumerated() {
            if index == 0 {
                polygonPath.move(to: polygonControlPointView.center)
            }
            let nextIndex = (index + 1) % polygonControlPointViews.count
            let nextControlPoint = polygonControlPointViews[nextIndex].center
            polygonPath.addLine(to: nextControlPoint)

            let editPoint = GeometryHelper.getMiddlePoint(point1: polygonControlPointView.center, point2: polygonControlPointViews[nextIndex].center)
            addPolygonPointView(center: editPoint, type: .add)
            
            if showDistanceMarker {
                let nextCoordinate = currentPolygonVertexes[nextIndex].coordinate
                let markerDistance = GeometryHelper.getDistance(from: currentPolygonVertexes[index].coordinate, to: nextCoordinate).rounded()
                let markerLayer = drawDistanceMarkerLayer(centerPoint: editPoint, text: String(format: "%.0f", markerDistance) + "m")
                distanceMarkerLayers.append(markerLayer)
            }
        }
        polygonPath.close()
        
        polygonLayer.removeFromSuperlayer()
        polygonLayer.path = polygonPath.cgPath
        polygonLayer.lineWidth = 1
        // 判断多边形是否合法,不合法则将线段以红色显示
        polygonLayer.strokeColor = isPolygonValid(index: currentIndex) ? polygonColor.cgColor : UIColor.red.cgColor
        polygonLayer.fillColor = polygonColor.cgColor
        polygonLayer.zPosition -= 1
        layer.addSublayer(polygonLayer)
        
        // 添加距离标记
        distanceMarkerLayer.sublayers?.removeAll()
        distanceMarkerLayer.removeFromSuperlayer()
        distanceMarkerLayers.forEach {
            distanceMarkerLayer.addSublayer($0)
        }
        distanceMarkerLayer.zPosition -= 1
        layer.addSublayer(distanceMarkerLayer)
    }

    private func drawDistanceMarkerLayer(centerPoint: CGPoint, text: String) -> CAShapeLayer {
        let textSize = getTextSize(text: text)
        let react = CGRect(x: centerPoint.x - 8, y: centerPoint.y - 8, width: textSize.width + 24, height: 16)
        let roundRectPath = UIBezierPath(roundedRect: react, cornerRadius: 8)
        let markerLayer = CAShapeLayer()
        markerLayer.path = roundRectPath.cgPath
        markerLayer.fillColor = UIColor.white.cgColor
        
        let textLayer = drawTextLayer(frame: CGRect(x: react.origin.x + 18, y: react.origin.y + (8 - textSize.height/2), width: textSize.width, height: textSize.height), text: text, foregroundColor: MeshColor.grey2, backgroundColor: UIColor.clear)
        markerLayer.addSublayer(textLayer)
        return markerLayer
    }
}
复制代码

上面这段代码非常明显的 bad smell 就是太长,大概有四十行。通常情况下一个方法长度超过 20 行意味着做了太多事。当然也有一些情况方法长一点是可以接受的。假设我们有一个抽屉,抽屉装的都是同一样东西,虽然把抽屉装满了,但是对于这个抽屉里装了什么还是一目了然。如果方法长,但是方法里只是单一的做类似的、很容易理解的事也可以接受。

上面代码第二个问题是代码中的抽象层次不一致。我举个例子,假设公司的 CEO 做了一个决策,他打算通知所有高管,然后高管再逐级同步给部门。但是 CEO 在通知完高管后,询问高管,这个决策你要通知的人有谁。高管说要通知 A、B、C。于是 CEO 在高管会上把 A、B、C 叫来告诉了他们这个决策。代码的抽象层级也是类似,本来在处理顶层的逻辑,接着代码直接去处理了下一层的细节。这样不同层级的代码在一个方法里会加大理解的难度。 现在我们开始一步步重构这段代码。

如果大家看了前面几篇地图的控件设计实现的文章,会发现这个方法还有一个结构上的问题。多边形的顶点位置是从 polygonEditPointViews 上取的。但是如果仔细思考一下,其实这个方法依赖的是顶点的位置,现在通过依赖 polygonEditPointViews 间接得到,这样多了不必要的依赖。多了这层不必要的依赖会增加代码的不稳定性,另外如果要隔离测试这个方法,隔离的代价也会更高。

那么我们首先做一个小改动,移除对 polygonEditPointViews 的依赖。可以修改方法的参数,把顶点坐标当做参数传进来。如果类的规模小,直接封装一个属性提供顶点坐标也可以。这里我选择比较直观的封装属性方式隔离。

class CustomMapOverlayView: UIView {  
   var polygonEditPointViews = [PolygonAnnotationView]()
   private var areaVertexs: [CGPoint] {
        return polygonControlPointViews.map { $0.center }
   }

    private func drawPolygon(currentIndex: Int = 0, showDistanceMarker: Bool = true) {
        guard areaVertexs.count >= 3 else { return }
        polygonEditPointViews.forEach { $0.removeFromSuperview() }
        polygonEditPointViews.removeAll()
        var distanceMarkerLayers = [CAShapeLayer]()
        let polygonPath = UIBezierPath()
        
        for (index, vertex) in areaVertexs.enumerated() {
            if index == 0 {
                polygonPath.move(to: vertex)
            }
            let nextIndex = (index + 1) % areaVertexs.count
            let nextControlPoint = areaVertexs[nextIndex]
            polygonPath.addLine(to: nextControlPoint)

            let editPoint = GeometryHelper.getMiddlePoint(point1: vertex, point2: areaVertexs[nextIndex])
            addPolygonPointView(center: editPoint, type: .add)
            
       		// ...	
            }
        }
		// ...
	}
}
复制代码

这样代码的可读性也好了一点,读的时候不要去关心 polygonEditPointViews。

这段代码主要做了三件事:绘制多边形,在多边形边的中点显示边距,在边上添加增加点的按钮。实现的时候三件事的实现细节又写在了一起。因此读起来感觉代码有多有乱。

我们首先隔离绘制多边形的代码。

    var polygonLayer = CAShapeLayer()
   
    private func drawPolygon(currentIndex: Int = 0, showDistanceMarker: Bool = true) {
        guard areaVertexs.count >= 3 else { return }
        renderPolygonLayer(changedPointIndex: currentIndex)
        polygonEditPointViews.forEach { $0.removeFromSuperview() }
        polygonEditPointViews.removeAll()
        var distanceMarkerLayers = [CAShapeLayer]()
        for (index, vertex) in areaVertexs.enumerated() {
            let nextIndex = (index + 1) % areaVertexs.count
            let editPoint = GeometryHelper.getMiddlePoint(point1: vertex, point2: areaVertexs[nextIndex])
            addPolygonPointView(center: editPoint, type: .add)
            if showDistanceMarker {
                let nextCoordinate = currentPolygonVertexes[nextIndex].coordinate
                let markerDistance = GeometryHelper.getDistance(from: currentPolygonVertexes[index].coordinate, to: nextCoordinate).rounded()
                let markerLayer = drawDistanceMarkerLayer(centerPoint: editPoint, text: String(format: "%.0f", markerDistance) + "m")
                distanceMarkerLayers.append(markerLayer)
            }
        }
        // 添加距离标记
        distanceMarkerLayer.sublayers?.removeAll()
        distanceMarkerLayer.removeFromSuperlayer()
        distanceMarkerLayers.forEach {
            distanceMarkerLayer.addSublayer($0)
        }
        distanceMarkerLayer.zPosition -= 1
        layer.addSublayer(distanceMarkerLayer)
    }
    
    private func renderPolygonLayer(changedPointIndex: Int = 0) {
        let polygonPath = UIBezierPath()
        polygonPath.move(to: areaVertexs[0])
        for index in 1 ..< areaVertexs.count {
            let nextIndex = (index + 1) % areaVertexs.count
            let nextControlPoint = areaVertexs[nextIndex]
            polygonPath.addLine(to: nextControlPoint)
        }
        polygonPath.close()

        polygonLayer.removeFromSuperlayer()
        polygonLayer.path = polygonPath.cgPath
        polygonLayer.lineWidth = 1
        // 判断多边形是否合法,不合法则将线段以红色显示
        polygonLayer.strokeColor = isPolygonValid(index: changedPointIndex) ? polygonColor.cgColor : UIColor.red.cgColor
        polygonLayer.fillColor = polygonColor.cgColor
        polygonLayer.zPosition -= 1
        layer.addSublayer(polygonLayer)
    }
复制代码

把绘制多边形的代码抽离出来后逻辑已经清晰很多了。

接着我们先重构一下 drawDistanceMarkerLayer方法。这个方法有两个问题:

  • 方法的名字不恰当。这个方法的作用是创建了一个 layer,并没有 draw 这个动作。因此名字要修改,以免引起歧义。
  • 方法的参数不够好,将参数的处理细节暴露在了外面。这个方法被调用的地方只有一处,参数应该让调用的地方尽量简洁。字符格式的配置应该在方法内完成。

重构完成后调用的地方是这样的:

 let markerLayer = createDistanceMarkerLayer(centerPoint: editPoint, markerDistance: markerDistance)

 //原来的调用
 let markerLayer = drawDistanceMarkerLayer(centerPoint: editPoint, text: String(format: "%.0f", markerDistance) + "m")
复制代码

接着我们把距离标记再抽出来。

    private func drawPolygon(currentIndex: Int = 0, showDistanceMarker: Bool = true) {
        guard areaVertexs.count >= 3 else { return }
        renderPolygonLayer(changedPointIndex: currentIndex)
        polygonEditPointViews.forEach { $0.removeFromSuperview() }
        polygonEditPointViews.removeAll()
        for (index, vertex) in areaVertexs.enumerated() {
            let nextIndex = (index + 1) % areaVertexs.count
            let editPoint = GeometryHelper.getMiddlePoint(point1: vertex, point2: areaVertexs[nextIndex])
            addPolygonPointView(center: editPoint, type: .add)
        }
        if showDistanceMarker {
            renderDistanceMarkerLayer()
        }
    }
    
    private func renderDistanceMarkerLayer() {
        var distanceMarkerLayers = [CAShapeLayer]()
        for index in 0 ..< areaVertexs.count {
            let nextIndex = (index + 1) % areaVertexs.count
            let middlePoint = GeometryHelper.getMiddlePoint(point1: areaVertexs[index], point2: areaVertexs[nextIndex])
            let nextCoordinate = currentPolygonVertexes[nextIndex].coordinate
            let markerDistance = GeometryHelper.getDistance(from: currentPolygonVertexes[index].coordinate, to: nextCoordinate).rounded()
            let markerLayer = createDistanceMarkerLayer(centerPoint: middlePoint, markerDistance: markerDistance)
            distanceMarkerLayers.append(markerLayer)
        }
        // 添加距离标记
        distanceMarkerLayer.sublayers?.removeAll()
        distanceMarkerLayer.removeFromSuperlayer()
        distanceMarkerLayers.forEach {
            distanceMarkerLayer.addSublayer($0)
        }
        distanceMarkerLayer.zPosition -= 1
        layer.addSublayer(distanceMarkerLayer)
    }
复制代码

做完这一步 drawPolygon 里的代码行数已经很少了,只有不到 10 行。在这个体量下前面说到旧代码问题的第二点就比较明显了:中间的绘制增加点的按钮和其他的层次不同,绘制增加点直接把实现写在这里了,抽象层次直接降低了。一个顶层方法应该负责调度,细节的实现不应该在里面。

最后我们把绘制增加点的按钮抽离出来。

    private func drawPolygon(currentIndex: Int = 0, showDistanceMarker: Bool = true) {
        guard areaVertexs.count >= 3 else { return }
        renderPolygonLayer(changedPointIndex: currentIndex)
        renderEditPoints()
        if showDistanceMarker {
            renderDistanceMarkerLayer()
        }
    }
    
    private func renderEditPoints() {
        polygonEditPointViews.forEach { $0.removeFromSuperview() }
        polygonEditPointViews.removeAll()
        
        for (index, vertex) in areaVertexs.enumerated() {
            let nextIndex = (index + 1) % areaVertexs.count
            let editPoint = GeometryHelper.getMiddlePoint(point1: vertex, point2: areaVertexs[nextIndex])
            let polygonPoint = createPolygonPoint(center: editPoint, type: .add)
            addSubview(polygonPoint)
            polygonEditPointViews.append(polygonPoint)
        }
    }
复制代码

完成后核心方法 drawPolygon 只有 5 行代码,这个方法做了什么应该非常清晰易理解了。子方法中负责各自绘制的部分。如果后期要绘制其他元素,在 drawPolygon 中增加。如果元素的 UI 有变化,到各个负责具体绘制的方法中修改也不会影响到其他模块。

重构的指导思想是什么?按照一种逻辑整理划分代码,把每块代码的体量控制在一个容易理解的范围里。