likes
comments
collection
share

过度设计: 接口方法不合理套用设计模式,导致简单问题复杂化

作者站长头像
站长
· 阅读数 4

@TOC

注意⚠️ 本文描述的设计并不合理,可以当做反面案例,下面纯属个人反思。

问题背景

模型搜索算法侧召回出现了badCase,需要对其进行问题排查,以往的人工排查流程划分了很多步骤,现在服务端需要把每一个step的返回值情况串联起来,获得最终的排查结果,流程图结构如下。 过度设计: 接口方法不合理套用设计模式,导致简单问题复杂化


一、基本功能实现

根据流程图所示的描述,创建了一个名称为GroundingSerive的接口和对应实现类,一个核心方法 badCaseDetect( GroundingDTO userInputParam ),在方法内部把流程图描述的所有步骤串联起来。由于步骤真的很多,外加大量if-else结果分支校验的判断,这个方法的代码行数最后接近100行,看上去结构混乱复杂并且可读性差。

修改之前的最初版本代码情况大概是下面这个样子: 过度设计: 接口方法不合理套用设计模式,导致简单问题复杂化 可以看到代码从59到139行,一共80行代码,如果严格按照流程图的描述去实现,由于每一个步骤都是不可或缺的,势必会出现if和else里面继续嵌套小的if-else分支判定的情况,使得判断的逻辑代码耦合性很强,难以拆分和优化,并且对于每一个步骤的流程执行情况很难分析,需要通过日志或者断点辅助走查代码逻辑才能判断这个方法内部具体执行到哪一个步骤了。

二、分析代码特点和存在的问题

分析之前的根据流程图的面向过程代码逻辑实现,总结出其有以下特点: 1.代码结构混乱,大量if-else出现,并且内部嵌套了小的if-else,步骤之间存在状态关联,代码逻辑严重耦合,难以修改和维护。 2.代码可读性差,需要走查逻辑根据断点和日志进一步确定执行情况,给其他同事分析流程造成困难和阻碍,不利于沟通协作。 3.整体来看,每一个步骤节点都具备公有逻辑,都可以根据用户输入参数判断当前节点的执行结果,传递给下一层进行进一步处理,理论上可以抽象成上下文,实现代码封装和复用。

因此我们首先可以想到的是,对于这种每一个步骤的共有逻辑进行封装,但是仔细观察发现,步骤之间是有逻辑判断耦合的,所以需要把耦合的部分从当前if-else关联的结构中拆分出来。比如step1这个步骤,执行成功以后继续判断步骤A2/A3, 失败以后继续执行B2/B3/B4, 原来的结构是 if(success) -> { A2 A3}, else ->{B2 B3 B4} ,导致步骤1的结果后面又嵌套了后续的流程,现在我们把它拆分出来,变成通用的逻辑step1->A2->A3 或者 step1->B2->B3->B4;

三、单纯套用状态模式修改代码

针对上述代码特点的分析,为了把每一个状态解耦出来单独负责各自的判断任务,我们可以联想到几种设计模式。

1.少量同等级的状态,如if- else if - else 这种结构,可以用工厂或者策略模式来进行动态调用; 2.单一链路每一部分各司其职,多个处理者之间依次传递的情况,可以考虑责任链模式,类似于Sentinel限流底层的设计; 3.逻辑上存在前后判定关系,多分支和变化,存在状态之间相互转换的耦合情况,我们可以考虑将步骤抽象成状态,考虑用状态模式完成状态转换以及中途结果判定。

综合对比之下我们采用状态模式,于是接下来进行具体改造:

(1) 抽象状态节点

我们把每一个判定步骤抽象成一个状态类State,每一个状态类有两个属性:这个状态的下一个状态nextState和上一个状态的执行结果lastResult。另外加一个execute抽象方法,其内部可以根据lastResult和当前的执行结果设置下一个状态节点。

public abstract class State {
    public State nextState;

    public FinalResult lastResult;

    abstract public FinalResult execute(GroundingService groundingService, GroundingDTO groundingDTO, FinalResult finalResult);

    public State getNextState() {
        return nextState;
    }

    public void setNextState(State nextState) {
        this.nextState = nextState;
    }

    public FinalResult getNextStepLastResult() {
        return lastResult;
    }

    public void setNextStepLastResult(FinalResult lastResult) {
        this.lastResult = lastResult;
    }
}

(2) 定义具体状态节点

我们有7个节点判断,所以可以让他们都继承抽象公共类State,内部实现各自的execute方法,其内部可以根据lastResult和当前的执行结果设置下一个状态节点。 过度设计: 接口方法不合理套用设计模式,导致简单问题复杂化

(2) 定义状态机

包括初始化状态的构造函数,一个属性currentState表示当前的状态,以及一个execute方法,方法内部具体执行的逻辑是:执行当前step的判断逻辑,获取执行结果,获取确定的下一个状态节点,并且设置其lastResult属性为当前的执行结果,继续迭代执行下一个状态节点的判断逻辑,直到nextState属性为null时候停止迭代。

public class StateMachine {
    private State currentState;
    
    public StateMachine(State initialState) {
        this.currentState = initialState;
    }

    public FinalResult execute(GroundingService groundingService, GroundingDTO groundingDTO) {
        FinalResult finalResult = new FinalResult();

        while (currentState != null) {
            finalResult = currentState.execute(groundingService,groundingDTO,currentState.lastResult); // 执行当前状态的 execute 方法并获取结果
            currentState = currentState.getNextState(); // 将当前状态设置为下一个状态(如果有的话)
            if(currentState!=null) currentState.lastResult = finalResult;
        }
        return finalResult;
    }
}

(3) 整理核心流程

这一部分抽象出来就是,初始化一个状态机,然后设置初始状态为Step1,然后逐步按照状态执行,根据结果转移到对应的下一个状态,直到当前状态节点的下一个状态为空,终止迭代,输出最终排查结果。 过度设计: 接口方法不合理套用设计模式,导致简单问题复杂化

经过上述3步修改,代码从原来的80行变成了7个状态类+1个状态机+1个不超过10行的核心方法的结构,结构是变得清晰了,实现了状态封装和逻辑流程解耦,重新测试代码,发现能够完全实现对应流程图的判定需求。

只不过这样做确实有点为了设计而设计,导致简单问题复杂化了,代码逻辑的可读性也降低了,其他同事要理解这个代码可能要花费一定的时间。后续尝试拆成独立的子方法,通过封装减少胶水代码,保留可读性,降低沟通成本。在此特别感谢评论区大佬的建议。


总结

对于某些涉及大量分支判定的复杂流程设计,可以考虑抽象成状态模型,利用状态模式优化单纯面向过程的代码结构,这种方式能够实现逻辑封装和复用,实现流程解耦,增强代码可读性,降低调用方项目技术沟通成本,使得接口方法架构更清晰明朗,但是设计模式的使用还是要区分场景,不要为了设计而设计,把简单问题复杂化。