一些实用的重构代码技巧

作者: Armstrong_Q | 来源:发表于2018-07-20 17:20 被阅读34次

目的

本文是来说明如何编写优秀的代码。好的代码不是一蹴而就，它需要不断重构，直到写出可读性，可测性，可拓展性的代码。

抛出代码的历史问题

这里用项目里面一个非常简单的功能做演示，引用了框架中对外的入口类
SmartHorizonSignalProvider。

可读性

下面列举了SmartHorizonSignalProvider的成员，可用发现类成员非常多，很难理解这个类的主要结构和依赖对象是什么。

Class Members

private static final int TIME_OUT_CHECK_INTERVAL = 5000;

private static final int TIME_OUT_LAST_MESSAGE_INTERVAL = 30000;

/**
* Flag that indicates if we are running the app on devices different than the HEAD UNIT.
*/
private boolean notOnHeadUnit;

/**
* adas message listener
*/
private AutoSdkNavigationService.AdasMessageListener adasMessageListener;

/**
* ADAS messages queue
*/
private Queue adasSmartHorizonMessageQueue;

/**
* broadcast thread
*/
private BroadcastThread broadcastThread;

/**
* value at which time frame the messages will be send
*/
private int frequencyValue;

/**
* last message received time
*/
private long lastMessageReceivedTime;

/**
* time out thread - used to stop the broadcasting process
*/
private TimeOutThread timeOutThread;

/**
* intent broadcast controller
*/
private IntentBroadcastController intentBroadcastController;

/**
* HeadUnit broadcast controller
*/
private HUBroadcastController huBroadcastController;

Method

下面这段方法，其中public方法和private方法混合调用，而且有结构很混乱，内部也违背了单一职责原则，它包括了多个操作，并不是startProvider()方法名所说的启动一个提供者。步骤：

读取系统属性.
判断系统设置是否打开.
如果设置打开，则注册监听器.

public void startProvider() {
        AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis startProvider");
        String smartHorizonFrequencyString = ApplicationContextHelper.getInstance().getConfigSettings(ApplicationContextHelper.KEY_ADAS_SMART_HORIZON_FREQUENCY);
        if (smartHorizonFrequencyString != null) {
            try {
                int frequencyValue = Integer.parseInt(smartHorizonFrequencyString);
                if (frequencyValue <= 0) {
                    this.frequencyValue = 20;
                } else {
                    this.frequencyValue = frequencyValue;
                }
            } catch (NumberFormatException e) {
                this.frequencyValue = 20;
            }
        }
 
        notOnHeadUnit = !ApplicationContextHelper.getInstance().isGMHeadUnit();
        if (notOnHeadUnit) {
            intentBroadcastController = new IntentBroadcastController();
        } else {
            huBroadcastController = new HUBroadcastController();
        }
 
        addAdasMessageListener();
}

public API name

在SmartHorizonSignalProvider类中，有个两个公共方法startProvider()、stopProvider()，但是类名中已经表示是provider这个动作，所以这里显得有点多余，用start\stop()命名就可以了。纵观上下文，BoradcaseControl、timeOutThread这样的命名也难以理解。

public void startProvider();
public void stopProvider();

可测试性

在SmartHorizonSignalProvider类是很难测试的，理由包括：
1.依赖于具体实现类，不容易被外部拓展，也难以被mock.
2.方法直接操作单例类.
3.内部类直接在类里面就被初始化.

可拓展性

在编写可扩展性代码一般是按照SOLID原则进行.
S- Single principle单一职责原则SmartHorizonSignalProvider这个类做了太多事，不止一件。如定义线程、开启线程、关闭线程、接收ADSD消息等。

/**
 * start time out thread
 */
private void startTimeOutThread() {
    AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis startTimeOutThread");
    timeOutThread = new TimeOutThread(TIME_OUT_CHECK_INTERVAL);
    timeOutThread.start();
}
 
/**
 * stop threads
 */
private void stopThreads() {
    AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis stopThreads");
    if (broadcastThread != null) {
        broadcastThread.cancel();
        broadcastThread = null;
    }
    if (timeOutThread != null) {
        timeOutThread.cancel();
        timeOutThread = null;
    }
}
 
/**
 * @param smartHorizonMessagesList List of SMART HORIZON messages received.
 */
private void smartHorizonMessageReceived(List<AdasMessage> smartHorizonMessagesList) {
    ...   
}
 
/**
 * handle adas messages queue
 */
private synchronized void handleMessagesQueue() {
    AdasMessage adasMessage = adasSmartHorizonMessageQueue.peek();
    if(adasMessage == null) {
        AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(),
                "adasis handleMessagesQueue NULL broadcast. Nothing to sent.");
        return;
    }
 
    AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(),
        "adasis handleMessagesQueue broadcast with messageSource = " + adasMessage.getMessageSource());
    broadcastMessage(adasMessage);
 
    adasSmartHorizonMessageQueue.poll();
    AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(),
            "adasis handleMessagesQueue new queue size = " + adasSmartHorizonMessageQueue.size());
}
 
/**
 * broadcast ADAS message
 *
 * @param message the ADAS message
 */
private void broadcastMessage(AdasMessage message) {
    if (notOnHeadUnit) {
        if (intentBroadcastController != null) {
            AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis broadcastMessage on TABLET. "
                    + "Message with type = " + message.getType() + ", source = " + message.getMessageSource()
                    + ", content = " + message.getContent());
            intentBroadcastController.handleMessage(message);
        } else {
            AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(),
                    "adasis broadcastMessage intentBroadcastController is NULL");
        }
    } else {
        if (huBroadcastController != null) {
            AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis broadcastMessage on HEAD UNIT. "
                    + "Message with type = " + message.getType() + ", source = " + message.getMessageSource()
                    + ", content = " + message.getContent());
            huBroadcastController.handleMessage(message);
        } else {
            AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(),
                    "adasis broadcastMessage huBroadcastController is NULL");
        }
    }
}
 
private class BroadcastThread extends Thread {
 
    /**
     * wait time value when trying to broadcast a new message
     */
    private int waitTime;
 
    /**
     * false if the thread should stop, true otherwise
     */
    private boolean notCancelled = true;
 
    public BroadcastThread(int waitTime) {
        AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, BroadcastThread.class,
                "adasis new BroadcastThread waitTime =  " + waitTime);
        this.waitTime = waitTime;
        adasSmartHorizonMessageQueue = new LinkedList<>();
        startTimeOutThread();
    }
 
    @Override
    public void run() {
        ...
    }
 
    /**
     * cancel thread
     */
    public void cancel() {
        AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis cancel thread");
        notCancelled = false;
    }
}
 
private class TimeOutThread extends Thread {
    /**
     * wait time when checking for last message received
     */
    private int waitTime;
 
    /**
     * false if the thread should stop, true otherwise
     */
    private boolean notCancelled = true;
 
    public TimeOutThread(int waitTime) {
        AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis new TimeOutThread waitTime =  " + waitTime);
        this.waitTime = waitTime;
    }
 
    @Override
    public void run() {
        ....
    }
 
    /**
     * cancel thread
     */
    public void cancel() {
        AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis cancel thread");
        notCancelled = false;
    }
}

O- Open/Close principle 开闭原则
在这里当前package目录下，如果不是必要，没必要全部类都是public类型。另外，要想不修改现有的代码，修改一些默认的UI行为，需要通过暴露一个接口，让其他Client去实现接口，这里没有专门的接口支持这个方案。

L- Liskov substitution principle 里斯提夫替代原则
这里没有用到这个原则。

I - Interface segregation principle接口隔离原则
因为接口少，这里没有用到这个原则。

D- Dependency inversion principle 依赖倒转原则
依赖倒置原则这里也没有实现，这是由于依赖的是具体实现类，而不是接口。SmartHorizonSignalProvider引用了具体实现类intentBroadcastController和huBroadcastController。

private IntentBroadcastController intentBroadcastController;
     
private HUBroadcastController huBroadcastController;
  
private class BroadcastThread extends Thread {
 
        /**
         * wait time value when trying to broadcast a new message
         */
        private int waitTime;
 
        /**
         * false if the thread should stop, true otherwise
         */
        private boolean notCancelled = true;
 
        public BroadcastThread(int waitTime) {
            AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, BroadcastThread.class,
                    "adasis new BroadcastThread waitTime =  " + waitTime);
            this.waitTime = waitTime;
            adasSmartHorizonMessageQueue = new LinkedList<>();
            startTimeOutThread();
        }
 
        @Override
        public void run() {
            ...
        }
 
        /**
         * cancel thread
         */
        public void cancel() {
            AutoSdkLog.log(LogEnum.LogPriority.debug, ArpLogFilter.LOG_MODULE, this.getClass(), "adasis cancel thread");
            notCancelled = false;
        }
    }

而BoradCastControl中的handleMessage()也是用的具体实现。


public void handleMessage(AdasMessage message) {
 ....     
}

开始重构历史问题代码

可读性

我们依然从可读性开始改造SmartHorizonSignalProvider类的ClassMebers，首先让它的成员变量更加精简，私有化了构造函数，通过Builder类来初始化对象。同时也提供了默认的初始化属性。

/**
 *  Smart Horizon Provider is a proxy between OEM CAN bus interface and
 *  Telenav SmartHorizon ADAS solution. Requires OEM specific
 *  @see AdasMessageHandler implementation.
 *  Provider can be used as is on stock Android devices in which case default
 *  broadcast channel is Intent bus.
 */
public class SmartHorizonProvider {
    private final List<AdasMessageHandler> adasMessageHandlers;
    private final AdasMessageReceiver receiver;
    private Scheduler scheduler;
 
    private boolean hasStarted;
 
    private SmartHorizonProvider(Builder builder) {
        this.receiver = builder.receiver;
        this.adasMessageHandlers = Collections.unmodifiableList(builder.adasMessageHandlers);
        this.scheduler = builder.scheduler;
        this.scheduler.registerExecutor(new Executor() {
            @Override
            public void run() {
                // Peek ADAS message and broadcast
                for (AdasMessageHandler handler: adasMessageHandlers) {
                    handler.handleAdasMessage(receiver.getMessage());
                }
            }
        });
    }
 
    /**
     * Start provider
     * @param interval broadcast interval in milliseconds
     */
    public void start(long interval) {
        if (hasStarted || interval <= 0) {
            return;
        }
 
        receiver.start();
        hasStarted = scheduler.start(interval);
    }
 
    /**
     *  Stop provider
     */
    public void stop() {
        receiver.stop();
        scheduler.cancel();
        hasStarted = false;
    }
 
    @VisibleForTesting
    void setScheduler(Scheduler scheduler) {
        this.scheduler = scheduler;
    }
 
    public static final class Builder {
        private static final int MAX_MESSAGES_IN_QUEUE = 20;
        private final List<AdasMessageHandler> adasMessageHandlers = new ArrayList<>();
        private final Scheduler scheduler = new BroadcastScheduler();
        private AdasMessageReceiver receiver;
 
        public Builder(@Nullable Context context) {
            /// Default initializations
            /// TODO: move this dependency upstream such that library is ARP SDK agnostic
            this.receiver = new ARPAdasMessageReceiver(AutoSdkNavigationService.getInstance(), MAX_MESSAGES_IN_QUEUE);
 
            /// Optional dependency
            if (context != null) {
                this.adasMessageHandlers.add(new BroadcastIntentTransmitter(context));
            }
        }
 
        /**
         * Override default ADAS receiver implementation
         * @param receiver reference to new receiver implementation
         * @return builder instance
         */
        public Builder setAdasMessageReceiver(@NonNull AdasMessageReceiver receiver) {
            this.receiver = receiver;
            return this;
        }
 
        /**
         * Add ADAS message handler (OEM specific)
         * @param messageHandler reference to Adas message handler
         * @return builder instance
         */
        public Builder addAdasMessageHandler(@NonNull AdasMessageHandler messageHandler) {
            if (adasMessageHandlers.contains(messageHandler)) {
                return this;
            }
 
            adasMessageHandlers.add(messageHandler);
            return this;
        }
 
        public SmartHorizonProvider build() {
            return new SmartHorizonProvider(this);
        }
    }
}

Method

创建了一个职责单一的接口，暴露了一个方法处理message。让api更加可读，同时写好了注释。

/**
 * Higher level abstraction for ADAS message handler
 * Concrete implementations are OEM specific
 * @param <T> generic ADAS message type
 */
public interface AdasMessageHandler<T> {
    /**
     * Handle ADAS message, null should be handled by client
     * before processing ADAS message, null implies no messages
     * are available for processing at the moment
     * @param message custom ADAS message
     */
    void handleAdasMessage(@Nullable T message);
}

可测试性

因为我们的SmartHorizonProvider依赖的对象都是面向接口，现在我们更加容易mock他们，来看看测试类如何写：

public class SmartHorizonProviderTest extends BaseTest {
    @Mock
    private AdasMessageHandler messageHandler;
    @Mock
    private AdasMessageReceiver messageReceiver;
    @Mock
    private Scheduler scheduler;
 
    SmartHorizonProvider smartHorizonProvider;
 
    @Before
    public void setup() {
        smartHorizonProvider = new SmartHorizonProvider.Builder(null)
                .addAdasMessageHandler(messageHandler)
                .setAdasMessageReceiver(messageReceiver)
                .build();
        smartHorizonProvider.setScheduler(scheduler);
    }
}

可拓展性

重构这个类的可扩展性，我们也通过SOLID原则进行。

Single Principle

现在我们的SmartHorizonProvider只负责开关provider一件事。另外以前的成员变量已修改成对应的类或接口去做单一的事情。

Open/Close Principle

我们这里已经有了两个接口一个private类，同时接口也提供了其他client在不改原有的代码前提下修改默认的行为。

Lusiko substitution Principle

这里我们不需要这个原则

Interface segregation principle

现在我们新定义了三个接口，专门负责三个独立的事情；

/**
 * Higher level abstraction for ADAS message handler
 * Concrete implementations are OEM specific
 * @param <T> generic ADAS message type
 */
public interface AdasMessageHandler<T> {
    /**
     * Handle ADAS message, null should be handled by client
     * before processing ADAS message, null implies no messages
     * are available for processing at the moment
     * @param message custom ADAS message
     */
    void handleAdasMessage(@Nullable T message);
}
  
/**
 * Higher level abstraction for ADAS message receiver
 * Receiver is bound to a specific implementation of ADAS daemon
 * @param <T> ADAS message type, implementation specific
 */
public interface AdasMessageReceiver<T> {
    /**
     * Start receiver, normally that's where initialization
     * of receiver should happen
     */
    void start();
 
    /**
     * Stop receiver, normally that's where cleanup state
     * of receiver should happen
     */
    void stop();
 
    /**
     * Return latest ADAS message, if message is unavailable return null
     * @return latest ADAS message
     */
    @Nullable T getMessage();
}
  
/**
 * Higher level abstraction for scheduler
 */
interface Scheduler {
    /**
     * Register executor callback, code that
     * needs to be executed periodically
     * @param executor reference to executor
     */
    void registerExecutor(@NonNull Executor executor);
 
    /**
     * Start scheduler
     * @param interval in millisecondss
     * @return true if scheduler is started successfully
     */
    boolean start(long interval);
 
    /**
     * Cancel scheduler
     */
    void cancel();
 
    interface Executor {
        /**
         *  Execute block of code
         */
        void run();
    }
}

Dependency inversion Princinle

我们现在的code没有直接依赖实现类，而是依赖抽象类。如果我们在不同的平台使用AdasMessageReceiver ，scheduler，现在我们只需要通过Builder注入新的实现类，不需要改变SmartHorizonProvider原有的内部逻辑。

    private final AdasMessageReceiver receiver;
    private Scheduler scheduler;

其他可用的技巧

在模块之间用不可变量的数据进行交互。
根据可扩展性的原则，我们要仔细设计公共类，为将来的重构打好基础。
不要暴露过多的public方法，如果没有特殊，可将构造函数私有化，多用Builder来初始化成员变量。
每次添加了新的代码，应该注意保证单元测试覆盖量。
不要期待一次性就能写到不需要重构的代码，而是要逐步迭代改进它。

一些实用的重构代码技巧

目的

抛出代码的历史问题

可读性

Class Members

Method

public API name

可测试性

可拓展性

开始重构历史问题代码

可读性

Method

可测试性

可拓展性

Single Principle

Open/Close Principle

Lusiko substitution Principle

Interface segregation principle

Dependency inversion Princinle

其他可用的技巧

相关文章

网友评论

延伸阅读

深度阅读

栏目导航

热点阅读

Android开发经验谈

better