■

今日も電気系の設計者は休みで動作試験ができないため、一日中リファクタリング。
１６文字×２行の液晶表示器(LCD)の制御がひどいことになっていたので整理した。なにがひどいかと言うと、ソースの方々で液晶に文字を表示するために直接ビットを操作していること。しかもそのたびにビットシフトやらマスクの計算までしている。
つまり、

   GPIO1 |= 0x03;                   //書き込みモード
   GPIO0 = 0xFF000000 + ('a'<<16);  //文字 'a' を表示
   GPIO1 |= 0x01;                   //STROBE ON
   GPIO1 &= ~0x01;                  //STROBE OFF
    ....

   char buff[] = "Hello World.":
    ....
   
   //文字列を表示
   GPIO1 |= 0x30;                  //書き込みモード
   for (i = 0; i < strlen(buff); ++i) {
       GPIO0 = 0xFF000000 + (buff[i]<<16);  //文字を表示
       GPIO1 |= 0x01;                       //STROBE ON
       GPIO1 &= ~0x01;                      //STROBE OFF
   }

みたいなコードがソースのあちこちに重複して存在している。
言うまでもないが、こういう場合は関数にまとめてしまうべき。たとえば、

void strobe_LCD(void)
{
    GPIO1 |= 0x01;                      //STROBE ON
    GPIO1 &= ~0x01;                     //STROBE OFF
}

void write_LCD_char(char c)
{
    GPIO1 |= 0x30;                  //書き込みモード
    GPIO0 = 0xFF000000 + (c<<16);
    strobe_LCD();
}

void write_LCD_str(const char* str)
{
    int i;
    GPIO1 |= 0x30;                  //書き込みモード
    for (i = 0; i < strlen(str); ++i) {
        GPIO0 = 0xFF000000 + (str[i]<<16);
        strobe_LCD();
    }
}

などのように機能ごとにまとめる。こうすれば文字を表示するコードの可読性と信頼性（コピペのミスなどが無くなるため）ははるかに上がる。

    write_LCD_char('a');
    void write_LCD_str("Hello world.");

機能をまとめてコードの重複を減らすことは基本中の基本だと思うのだが、このソースではあまりそれが出来ていない。たまりかねて、元のソースを書いたプログラマに、なぜ関数にまとめていないのか聞いたことがある。
「もともとアセンブラで書いていたから」というのがそのときの解答だったが、これは理由にはならない。アセンブラでもCALL命令を使って関数に近いことはできる。