Codelogy

コード添削

松浦の記事、【c++】インタプリタを初めから丁寧に 第02回 のコードが、ちょっと冗長な気がしますので、これを添削してみようかと思います。

まずは、識別子の解析をする部分から。 (もとのコードはこちらを参照してください。)

#include <ctype.h>

std::string str; // スクリプトの内容
static int Pos;  //読み取り位置

bool ReadIdentifier(){

    int n =Pos;

    // 1文字目 (英字 or アンダースコア)
    if (isascii(str[n]) && (isalpha(str[n]) || str[n] == '_'))
        ++n;
    else
        return false;
 
    // 2文字目以降 (英数字 or アンダースコア)
    while (isascii(str[n]) && (isalnum(str[n]) || str[n] == '_')) ++n;

    //識別子を格納
    std::string identify =str.substr(Pos, Pos - n);

    //読み取り位置を更新
    Pos =n;

   return true;
}
1. 関数名
もとの関数名は ReadIdentify ですが、識別子は "identifier" なので、関数名もこれに併せて修正しました。
2. 読み込みの位置
もとのコードでは、関数内で ReadLine を用いてテキストデータを読み込んでいましたが、これは不適切。
これでは、空行に対応できませんし、1行に複数のトークンがある場合も上手く動作しません。(str を上書きしてしまっているから。)
プログラムの全体的な作りからして、この関数は str に充分な長さのテキスト (スクリプト) を読み込んでから呼び出す、といった使い方をする必要があります。
そのため、内部での読み込みは廃止しました。
3. 標準関数を使おう
C言語では、isalpha, isalnum といった関数が標準ライブラリにより提供されおり、これを使わない手はありません
また、これらの関数はテーブル参照で実装されているため、< <= 演算子を用いての比較よりもパフォーマンスに優れます。
4. ループと終了条件
もとのコードでは、2文字目以降を読み込むループの終了条件が分かりづらくなっています。
そもそも、空白 (' ') 以外の文字での終了が認められないことになっていますが、これは不適切です。(data=10; などに対応できない。)
修正版では、while ループを、識別子の要素となる文字が続いているうちは続行される、という自然な形式に修正しました。

さて、次は整数および文字列リテラルをする部分です。(もとのコード)

//整数リテラルの読み取り
bool ReadLiteralInteger(){ 

    int n = Pos;

    // 1文字目
    if (!isdigit(str[n]))
        ++n;
    else
        return false;

    // 2文字目以降
    while (isdigit(str[n])) ++n;

    //リテラルの表記および値を登録
    string desc  = str.substr(Pos, n - Pos);
    int    value =atoi(desc);

    //読み取り位置を更新
    Pos =n;

    return true;
}

//文字列リテラルの読み取り
bool ReadLiteralString() { 

    int n =Pos;
    std::string content ="";

    if (str[n] == '\"' )
        ++n;
    else
        return false; 

    //バックスラッシュ (\) によるエスケープ状態
    bool bEscaped =false;

    for (;;) {

        if (bEscaped){
            bEscaped =false;
            ++n;
        }
        else {

            if (str[n] == '\"'){
              ++n;
              break;
            }

            if (str[n] == '\\'){
                bEscaped =true;
                ++n;
            }
            else {

                unsigned char c =str[n];

                // 2バイト文字チェック
                if ((0x81 <= c && c <= 0x9F) || (0xE0 <= c && c <= 0xFC))
                    n +=2;
                else
                    ++n;
            }
        }
    } // for (;;)

    std::string desc =str.substr(n, Pos);

    //読み取り位置を更新
    Pos =n;

    return true;
}

修正に関して留意すべき点は、前掲のコードとほぼ同じです。 とくに挙げるとすれば、

1. 標準関数を使おう
C言語では、atoi という関数が標準ライブラリにより提供され (以下略)
2. 同じ式を何度も書かない
もとのコードでは、2バイト文字の判定をするために、str[n]unsigned char 型にキャストする、という記述を 4回も繰り返しています。
こうした冗長さは、コードの可読性を下げるだけでなく、修正のし忘れを誘発しやすいといった欠点があります。

複雑なプログラムを見た際、「すげー」ではなく「めんどくせー」と思えるようになれば、初心者から一歩進んだと言えるかもしれません。

担当: 成田 (シンプルがいい!)

コメントを投稿

コメントの公開は承認制のため、投稿から掲載までに時間がかかることがあります。


About

2007年11月17日 14:00 に投稿されたエントリです。

他にも多くのエントリがあります。
メインページアーカイブページもご覧ください。