note for "The Art of Readable Code" - CH9 Variables and Readability

減少不必要的變數可以增加閱讀性, 比如

root_message.last_view_time = datetime.datetime.now()
會比下面code更容易理解
now = datetime.datetime.now()
root_message.last_view_time = now

下面還有幾個需要優化的的例子, 可以好好思考如何改善.

boolean done = false;
while (/* condition */ && !done) {
    ...
    if (...) {
        done = true;
        continue;
    }
}
與
var remove_one = function (array, value_to_remove) {
    var index_to_remove = null;
    for (var i = 0; i < array.length; i += 1) {
        if (array[i] === value_to_remove) {
            index_to_remove = i;
            break;
        }
    }
    if (index_to_remove !== null) {
        array.splice(index_to_remove, 1); 
    }
};

programmer都知道要盡量限縮變數的範圍, 因為可視範圍小, 要記住的變數數量也會減少, 也且可以避免global/local variable用錯的窘境.

submitted = false; // Note: global variable
var submit_form = function (form_name) {
    if (submitted) {
        return;  // don't double-submit the form
    }
    ...
    submitted = true;
};
可以被修改成
var submit_form = (function () {
    var submitted = false; // Note: can only be accessed by the function below
    return function (form_name) {
        if (submitted) {
            return;  // don't double-submit the form
        }
        ...
        submitted = true; 
    };
}());

參考資料:
1. The Art of Readable Code

Table Of Content for tag "The Art of Readable Code"

這是一本好書, 建議每個programmer都應該買來翻一翻

• CH1, Code Should Be Easy to Understand
• CH2, Pack information into your names
• CH3, Names That Can’t Be Misconstrued
• CH4, Aesthetics
• CH5, Knowing What to Comment
• CH6, Making Comments Precise and Compact
• CH7, Making Control Flow Easy to Read
• CH8, Breaking Down Giant Expressions
• CH9, Variables and Readability
• CH10, Extracting Unrelated Subproblems
• CH11, One Task at a Time
• CH12 Turning Thoughts into Code
• CH13 Writing Less Code

An Sample Code for notification-chain

這個pattern設計是根據kernel的notification chain做一些修改, 主要應用於眾多subscriber註冊於某個有興趣的event ID, 會根據註冊時的權重由大到小串成link-list, 當publisher執行blocking_notif_call(event_id, int val, void *v)時, 就會依序call這些subscriber註冊的callback function, 達到通知的效果.

notification-chain.h

#ifndef NOTIFICATION_CHAIN_H
#define NOTIFICATION_CHAIN_H

enum notif_id {
    /* 新的ID請加在這裡 */
    notif_id_network,
    notif_id_lte,
    notif_id_max /* must be last */
};

int notif_init(void);

typedef int (*notif_call_fp)(char const * caller, unsigned long val, void *v);

int _notif_reg(enum notif_id nid,
                notif_call_fp notif_call,
                char *notif_call_name,
                int priority);
#define notif_reg(nid, notif_call, priority) _notif_reg(nid, notif_call, #notif_call, priority);
int notif_unreg(enum notif_id nid, notif_call_fp notif_call);
char * notif_dump(char *buf, int sz);

/* call in the caller thread */
int _blocking_notif_call(enum notif_id nid,
                        unsigned long val, /* Value passed to notifier function */
                        void *v, /* Pointer passed to notifier function */
                        char const * caller);

#define blocking_notif_call(nid, val, v) _blocking_notif_call(nid, val, v, __FUNCTION__)

/* call by another thread */
int async_notif_call(enum notif_id nid,
                    unsigned long val /* Value passed to notifier function */,
                    void *v /* Pointer passed to notifier function */,
                    void (*free_v)(void *v));

#endif

notification-chain.c

#include <pthread.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <ctype.h>
#include <time.h>
#include <sys/time.h>
#include <sys/timerfd.h>
#include <stdint.h>
#include <unistd.h>
#include <pthread.h>
#include "notification-chain.h"

struct notif_block {
    notif_call_fp notif_call;
    char *notif_call_name;
    struct notif_block *next;
    int priority;
    int called;
};

struct notif_chain_head {
    pthread_mutex_t mutex;
    pthread_cond_t cond;
    struct notif_block *next;
};

struct notif_statistical {
    unsigned int alloc;
    unsigned int alloced;
    unsigned int freed;
};

struct notif_statistical notif_statistical = {};

static struct notif_chain_head *_notif_chain_tab = NULL;


static int _init_notif_chain(struct notif_chain_head **h)
{
    int i;

    *h = (struct notif_chain_head *) malloc(sizeof(struct notif_chain_head) * notif_id_max);
    if (!*h) {
        return -1;
    }

    for (i = 0; i < notif_id_max; i++) {
        pthread_mutex_init(&((*h)[i].mutex), NULL);
        pthread_cond_init(&((*h)[i].cond), NULL);
        (*h)[i].next = NULL;
    }
    return 0;
}

int notif_init(void)
{
    int ret = 0;

    if ((ret = _init_notif_chain(&_notif_chain_tab)) < 0) {
        return ret;
    }
    return ret;
}

static int _notif_check_parms(enum notif_id nid,
                            notif_call_fp notif_call,
                            char *notif_call_name)
{
    if (!_notif_chain_tab) {
        return -1;
    }

    if ((nid < 0) || (nid >= notif_id_max)) {
        return -1;
    }

    if (!notif_call) {
        return -1;
    }

    if (!notif_call_name) {
        return -1;
    }

    return 0;
}

static int _notif_block_alloc(struct notif_block **n)
{
    notif_statistical.alloc++;
    *n = (struct notif_block*) malloc(sizeof(struct notif_block));
    if (!(*n)) {
        return -1;
    }
    memset(*n, 0, sizeof(struct notif_block));
    notif_statistical.alloced++;
    return 0;
}

static int _notif_block_free(struct notif_block *n)
{
    notif_statistical.freed++;
    if (n->notif_call_name) {
        free(n->notif_call_name);
    }
    if (n) {
        free(n);
    }
    return 0;
}

int _notif_reg(enum notif_id nid,
    notif_call_fp notif_call,
    char *notif_call_name,
    int priority)
{
    struct notif_block **n, *nb;

    if (_notif_check_parms(nid, notif_call, notif_call_name) < 0) {
        return -1;
    }

    if (_notif_block_alloc(&nb) < 0) {
        return -1;
    }

    /* fill info */
    nb->notif_call = notif_call;
    nb->notif_call_name = strdup(notif_call_name);
    if (!nb->notif_call_name) {
        _notif_block_free(nb);
        return -1;
    }
    nb->priority = priority;

    /* insert into tab */
    pthread_mutex_lock(&(_notif_chain_tab[nid].mutex));
    n = &(_notif_chain_tab[nid].next);
    while ((*n) && ((*n)->priority > nb->priority)) {
        n = &((*n)->next);
    }
    nb->next = *n;
    *n = nb;
    pthread_mutex_unlock(&(_notif_chain_tab[nid].mutex));


    return 0;
}

int notif_unreg(enum notif_id nid, notif_call_fp notif_call)
{
    struct notif_block **n, *sn = NULL;

    if (_notif_check_parms(nid, notif_call, (void *) 0xFF /* ignore check */) < 0) {
        return -1;
    }

    pthread_mutex_lock(&(_notif_chain_tab[nid].mutex));
    n = &(_notif_chain_tab[nid].next);
    while ((*n)) {
        if ((*n)->notif_call == notif_call) {
            sn = *n;
            *n = (*n)->next;
            break;
        }
        n = &((*n)->next);
    }
    pthread_mutex_unlock(&(_notif_chain_tab[nid].mutex));

    if (!sn) {
        // not found
        return -1;
    }

    _notif_block_free(sn);

    return 0;
}
/* call in the caller thread */
int _blocking_notif_call(enum notif_id nid,
                        unsigned long val, /* Value passed to notifier function */
                        void *v, /* Pointer passed to notifier function */
                        char const * caller)
{
    struct notif_block *n;

    if (_notif_check_parms(nid, (void*) 0xFF /* just ignore check */, (void *) 0xFF /* just ignore check */) < 0) {
        return -1;
    }

    /* insert into tab */
    pthread_mutex_lock(&(_notif_chain_tab[nid].mutex));
    n = _notif_chain_tab[nid].next;
    while (n) {
        n->notif_call(caller, val, v);
        n->called++;
        n = n->next;
    }
    pthread_mutex_unlock(&(_notif_chain_tab[nid].mutex));

    return 0;
}
char * notif_dump(char *buf, int sz)
{
    int i;
    char *p = buf;
    struct notif_block *n;

    memset(buf, 0, sz);
    p += snprintf(p, sz - (buf - p), "\tnotif_fp/notif_fp_name/pri called\n");
    for (i = 0; i < notif_id_max; i++) {
        pthread_mutex_lock(&(_notif_chain_tab[i].mutex));
        n = _notif_chain_tab[i].next;
        p += snprintf(p, sz - (buf - p), "notif id: %d\n", i);
        while (n) {
            p += snprintf(p, sz - (buf - p), "\t%p/%s/%d %d\n",
                            n->notif_call, n->notif_call_name, n->priority, n->called);
            n = n->next;
        }
        pthread_mutex_unlock(&(_notif_chain_tab[i].mutex));
    }
    return buf;
}

main.c

#include <stdio.h>
#include <unistd.h>
#include "notification-chain.h"
/* 這裡有用service-reg作為publisher */
#include "service-reg.h"

#define LINK_DOWN   0
#define LINK_UP     1

void *publisher(void *v)
{
    int *link_state = (int *) v;
    ++(*link_state);
    (*link_state) %= 2;
    blocking_notif_call(notif_id_network, *link_state, NULL);
}

int link_notif_call1(char const *caller, unsigned long val, void *v)
{
    printf("%s(#%d): caller:%s, %s\n",
                    __FUNCTION__, __LINE__, caller, val == LINK_UP ? "UP" : "DOWN");
    return 0;
}

int link_notif_call2(char const *caller, unsigned long val, void *v)
{
    printf("%s(#%d): caller:%s, %s\n",
                    __FUNCTION__, __LINE__, caller, val == LINK_UP ? "UP" : "DOWN");
    return 0;
}

int link_notif_call3(char const *caller, unsigned long val, void *v)
{
    printf("%s(#%d): caller:%s, %s\n",
                    __FUNCTION__, __LINE__, caller, val == LINK_UP ? "UP" : "DOWN");
    return 0;
}

int main(int argc, char *argv[])
{
    int link_state = 0;
    char buf[1024];
    /* 請先call notif_init()做初始化 */ 
    notif_init();

    srv_reg("publisher", publisher, &link_state);
    srv_start_periodical("publisher", 1, 0);
    sleep(2);
    printf("reg 1, 10\n");
    notif_reg(notif_id_network, link_notif_call1, 10);

    sleep(2);
    printf("dump\n");
    printf("%s\n", notif_dump(buf, sizeof(buf)));

    sleep(2);
    printf("reg 3, 30\n");
    notif_reg(notif_id_network, link_notif_call3, 30);

    sleep(2);
    printf("reg 2, 20\n");
    notif_reg(notif_id_network, link_notif_call2, 20);

    sleep(2);
    printf("dump\n");
    printf("%s\n", notif_dump(buf, sizeof(buf)));

    sleep(2);
    printf("unreg 3\n");
    notif_unreg(notif_id_network, link_notif_call3);

    sleep(2);
    printf("dump\n");
    printf("%s\n", notif_dump(buf, sizeof(buf)));

    sleep(2);
    printf("unreg 1 lte \n");
    notif_unreg(notif_id_lte, link_notif_call1);

    sleep(2);
    printf("dump\n");
    printf("%s\n", notif_dump(buf, sizeof(buf)));

    return 0;
}

result

brook@vista:~/notification-chain$ ./demo
reg 1, 10
link_notif_call1(#20): caller:publisher, UP
link_notif_call1(#20): caller:publisher, DOWN
dump
        notif_fp/notif_fp_name/pri      called
notif id: 0
        0x400fa4/link_notif_call1/10    2
notif id: 1

link_notif_call1(#20): caller:publisher, UP
link_notif_call1(#20): caller:publisher, DOWN
reg 3, 30
link_notif_call3(#34): caller:publisher, UP
link_notif_call1(#20): caller:publisher, UP
link_notif_call3(#34): caller:publisher, DOWN
link_notif_call1(#20): caller:publisher, DOWN
reg 2, 20
link_notif_call3(#34): caller:publisher, UP
link_notif_call2(#27): caller:publisher, UP
link_notif_call1(#20): caller:publisher, UP
link_notif_call3(#34): caller:publisher, DOWN
link_notif_call2(#27): caller:publisher, DOWN
link_notif_call1(#20): caller:publisher, DOWN
dump
        notif_fp/notif_fp_name/pri      called
notif id: 0
        0x401046/link_notif_call3/30    4
        0x400ff5/link_notif_call2/20    2
        0x400fa4/link_notif_call1/10    8
notif id: 1

link_notif_call3(#34): caller:publisher, UP
link_notif_call2(#27): caller:publisher, UP
link_notif_call1(#20): caller:publisher, UP
link_notif_call3(#34): caller:publisher, DOWN
link_notif_call2(#27): caller:publisher, DOWN
link_notif_call1(#20): caller:publisher, DOWN
unreg 3
link_notif_call2(#27): caller:publisher, UP
link_notif_call1(#20): caller:publisher, UP
link_notif_call2(#27): caller:publisher, DOWN
link_notif_call1(#20): caller:publisher, DOWN
dump
        notif_fp/notif_fp_name/pri      called
notif id: 0
        0x400ff5/link_notif_call2/20    6
        0x400fa4/link_notif_call1/10    12
notif id: 1

link_notif_call2(#27): caller:publisher, UP
link_notif_call1(#20): caller:publisher, UP
link_notif_call2(#27): caller:publisher, DOWN
link_notif_call1(#20): caller:publisher, DOWN
unreg 1 lte
link_notif_call2(#27): caller:publisher, UP
link_notif_call1(#20): caller:publisher, UP
link_notif_call2(#27): caller:publisher, DOWN
link_notif_call1(#20): caller:publisher, DOWN
dump
        notif_fp/notif_fp_name/pri      called
notif id: 0
        0x400ff5/link_notif_call2/20    10
        0x400fa4/link_notif_call1/10    16
notif id: 1