评价一下之前写的代码，可圈可点？烂得一批？

最新推荐文章于 2024-11-14 18:01:00 发布

拳拳小布

最新推荐文章于 2024-11-14 18:01:00 发布

阅读量135

点赞数

分类专栏： C语言|代码质量文章标签：单片机嵌入式硬件

本文链接：https://blog.csdn.net/quanquanxiaobu/article/details/116139985

版权

C语言|代码质量专栏收录该内容

10 篇文章 1 订阅

订阅专栏

文章目录

为什么要写这篇

前几天写了一篇代码质量提升之不写重复的代码，提到了我对写不重复代码做出的努力，仔细想想，还有一次重复代码的优化没有写出来。找个机会写一下，比如：以如今的能力与眼界去评价一下之前写的代码。

Multi_timer timer_rgbwaterlamp;

/* 流水灯函数回调 */
void rgb_water_lamp_callback(void)
{
    static uint8_t times = 0;
    static uint8_t flag = 0;

    if(flag_water_lamp)
    {
        if(flag == 0)
        {
            times++;
        }else
        {
            times--;
        }
        
        LED_all_no_display();
        switch(times)
        {
            case 1: led_1_set_color(RED); break;
            case 2: led_2_set_color(RED); break;
            case 3: led_3_set_color(RED); break;
            case 4: led_4_set_color(RED); break;
            case 5: led_5_set_color(RED); break;
            case 6: led_6_set_color(RED); break;
            case 7: led_1_set_color(YELLOW); break;
            case 8: led_2_set_color(YELLOW); break;
            case 9: led_3_set_color(YELLOW); break;
            case 10: led_4_set_color(YELLOW); break;
            case 11: led_5_set_color(YELLOW); break;
            case 12: led_6_set_color(YELLOW); break;
            case 13: led_1_set_color(GREEN); break;
            case 14: led_2_set_color(GREEN); break;
            case 15: led_3_set_color(GREEN); break;
            case 16: led_4_set_color(GREEN); break;
            case 17: led_5_set_color(GREEN); break;
            case 18: led_6_set_color(GREEN); break;
            case 19: led_1_set_color(CHING); break;
            case 20: led_2_set_color(CHING); break;
            case 21: led_3_set_color(CHING); break;
            case 22: led_4_set_color(CHING); break;
            case 23: led_5_set_color(CHING); break;
            case 24: led_6_set_color(CHING); break;
            case 25: led_1_set_color(BLUE); break;
            case 26: led_2_set_color(BLUE); break;
            case 27: led_3_set_color(BLUE); break;
            case 28: led_4_set_color(BLUE); break;
            case 29: led_5_set_color(BLUE); break;
            case 30: led_6_set_color(BLUE); break;
            case 31: led_1_set_color(PURPLE); break;
            case 32: led_2_set_color(PURPLE); break;
            case 33: led_3_set_color(PURPLE); break;
            case 34: led_4_set_color(PURPLE); break;
            case 35: led_5_set_color(PURPLE); break;
            case 36: led_6_set_color(PURPLE); break;
            case 37: led_1_set_color(WHITE); break;
            case 38: led_2_set_color(WHITE); break;
            case 39: led_3_set_color(WHITE); break;
            case 40: led_4_set_color(WHITE); break;
            case 41: led_5_set_color(WHITE); break;
            case 42: led_6_set_color(WHITE); break;
            default:                //当为0和43时，flag值变化，有一个间隔的空白
                if(flag == 0)
                {
                    flag = 1;
                }else
                {
                    flag = 0;
                }
                break;
        }
    }

}


/*
 * 功能描述：流水灯1---6个灯，从红灯--黄灯--绿灯--青灯--蓝灯--紫灯--白灯，再从白灯到红灯 一直循环
 * 备注：传入参数_time 不能为0
 */
void
rgb_water_lamp_set(uint8_t _time)
{
    if(_time)               /* 对于传入参数的要求进行验证 */
    {
        reset_to_gpio();	///< 回到原始的状态,包括所有模式标志位清除

        flag_water_lamp = true;
        multi_timer_init(&timer_rgbwaterlamp, rgb_water_lamp_callback, _time * 50, _time * 50);		//50ms定时中断1次
        multi_timer_start(&timer_rgbwaterlamp);
    }
}

如何评价？

嗯~ ，看起来还不错，比如

函数的命名基本上能一眼了解其用途，不至于再深入具体每一个函数了解其实现从而理解其用途。
将要实现的功能通过注释说明了，就这一点很多人都做不到，因为懒得整理代码
对传入参数进行验证，做得恰当的话不会让程序运行到未知之地，更不会引发“蝴蝶效应”
对于程序有认真分析过，“有一个间隔的空白”，这可不是从程序运行情况能看出来的

但是，这样的代码可读性好吗？just so so

首先，一个简单的功能写得函数这么长，必定有着重复，或者不止一个功能。重复需简化，多功能则函数可拆分。这个rgb_water_lamp_callback一看就包含特别多重复的语句。
重要的设置参数_time 竟然不能清楚地了解其含义（只能简单了解其用途，却不清楚它的用法，也没有说明性注释），就对调用产生影响。
一个if(flag_water_lamp) 这么长，活脱脱就像语文上的“一逗到底”（全篇没有一个句号，好几口气都读不完一句话）
软件定时器的设置不容易理解，两个“_time * 50” 是表示相同的含义，还是不同的含义？不同的含义却用相同的表示方法。

我打算怎么改？

除了上面提到的那几点外，还打算把注释都换成英文的，因为那Eclipse 导致我所有的注释都变成了乱码繁体字，真是当真能体会到“楚人一炬，付之焦土”的惋惜与悲凉。后来经过一番痛苦的解决无果和思想挣扎，决定之后尽可能不写中文注释了，因为编码方式稍微一变，就都成了乱码。被迫接触英语了，我想我会习惯的。

另外，过多的设置也容易出错，所以我将软件定时器设置时又简化了一点，将两句设置整合为一句。

typedef struct Multi_Timer  Soft_Timer;
typedef void (*Timeout_Cb)(void);

/**
 * software_timer_set
 * @brief   Set software timer
 * @param   handle Explain : timer handle
 * @param   callback_func Explain : callback function pointer
 * @param   timeout_ms Explain : Timing timeout_ms ms
 * @param   is_repeat Explain : This timer wether to repeat excute
 */
void
software_timer_set(Soft_Timer *handle, Timeout_Cb callback_func, uint32_t timeout_ms, _Bool is_repeat)
{
    if (is_repeat)
    {
        multi_timer_init(handle, callback_func, timeout_ms, timeout_ms);
        multi_timer_start(handle);
    }
    else
    {
        multi_timer_init(handle, callback_func, timeout_ms, 0);
        multi_timer_start(handle);
    }
}

/**
 * software_timer_stop
 * @brief   Software timer stop
 * @param   handle Explain : timer handle
 */
void
software_timer_stop(Soft_Timer *handle)
{
    multi_timer_stop(handle);
}


Soft_Timer timer_rgbwaterlamp;


static void
single_rgb_color_choose(uint8_t value)
{
    uint8_t set_color = (value / 6) + 1;
    uint8_t choose_led = (value % 6) + 1;
    
    switch(choose_led)
    {
        case 1: led_1_set_color(set_color); break;
        case 2: led_2_set_color(set_color); break;
        case 3: led_3_set_color(set_color); break;
        case 4: led_6_set_color(set_color); break;
        case 5: led_5_set_color(set_color); break;
        case 6: led_4_set_color(set_color); break;
        default: break;
    }
}

/**
 * rgb_water_lamp_callback
 * @brief 	timer callback : rgb water lamp
 * @note	1. Set up every time, start over
 * @note 	2. choose single rgb color
 * @note 	3. from red --> ... --> white, then white --> ... --> red, go round ang begin again
 */
void
rgb_water_lamp_callback(void)
{
    static uint8_t into_times = 0;
    static uint8_t state_change_flag = 1;

    if(flag_water_lamp)
    {
        flag_water_lamp = false;
        into_times = 0;
        state_change_flag = 1;
    }else
    {
        /* no code */
    }

    rgb_all_no_display_invoke();
    single_rgb_color_choose(into_times);
    
    if(state_change_flag % 2)
    {
        into_times++;
        if(into_times >= 41)
        {
            state_change_flag = 0;
        }
    }else
    {
        into_times--;
        if(into_times == 0)
        {
            state_change_flag = 1;
        }
    }
}


/**
 * rgb_water_lamp_set
 * @brief	Water lamp(1 ~ 6 RGB),from red --> yellow --> green --> cyan --> blue --> purple --> white, then white --> red , go round ang begin again
 * @details Setting the switching interval of water lamp (Unit : 50ms)
 * @attention The incoming parameter(u8time_50ms) cannot be 0
 * @param	u8time_50ms description: Set time (Range : 1~255) (Unit : 50ms)
 */
void
rgb_water_lamp_set(uint8_t u8time_50ms)
{
    if(u8time_50ms)
    {
        uint16_t time_50ms = u8time_50ms * 50;
        
        reset_to_gpio();	///< Return to the original state, including all mode flags clear 0  by quanquanxiaobu

        flag_water_lamp = true;
        software_timer_set(&timer_rgbwaterlamp, rgb_water_lamp_callback, time_50ms, true);
    }
}