コードリファクタリング変数の命名編

2025年2月24日2025年2月27日

久々に大学時代に使用していたPC内のソースコードを見てみた。

初期ソースコード

#include <stdio.h>

int main(void) {
    char sen[1000];
    char out;
    int i = 0;
    int flag;
    
    while(1){
        printf("暗号作成の時は0を\n暗号解読のときは1を\n終了するときは2を入力してください:");
        scanf("%d",&flag);
    
        if (flag == 0 || flag == 1) {
            if (flag == 0)
                printf("暗号化させたい文を入力してください\n");
            else
                printf("解読させたい文を入力してください\n");

            scanf("%s",sen);
        
            while (sen[i] != NULL) {
                if (sen[i] >= 65 && sen[i] <= 77)
                    out = sen[i] + 13;
                else if (sen[i] >= 78 && sen[i] <= 90)
                    out = sen[i] - 13;
                else if (sen[i] >= 97 && sen[i] <= 109)
                    out = sen[i] + 13;
                else if (sen[i] >= 110 && sen[i] <= 122)
                    out = sen[i] - 13;
                else out = sen[i];
                printf("%c",out);
                i++;
            }
            i = 0;
        }
        else if (flag == 2)
            return 0;
        else
            printf("正しい数字を入力してください(怒)");

        printf("\n");
    }
}

前提知識として、これはROT13と呼ばれるシーザー暗号の1つである。(https://ja.wikipedia.org/wiki/ROT13)
このWikipedia内の例にもあるが、「HELLO」という文字を入力すると、「H」の13文字後の「U」、「E」の13文字後の「R」…と入力した文字に対してその文字の13文字後に変換して出力するというものである。(答えは「URYYB」となる。)

ただ、このプログラムは読みづらく、ROT13が実装できているかパッと見では理解できない。
どこが読みづらいかを1つずつ掘り下げていく。

変数名の命名について

まずは変数名に注目する。

    char sen[1000];
    char out;
    int i = 0;
    int flag;

sen, out, i, flagという変数名が定義されているが、変数名だけで何を取り扱っているものかわからない。

今回のソースコードは40行程度であるので全行読んで変数の役割を理解することができる。
ただ、これが業務用のプログラムであり、ソースコードの量が数百~数千に到達した場合どうなるのか。
また、これを自分でない他の人が拡張対応する場合にどのようなことを考える可能性があるのか。

まずありそうなパターンは、senの中身を置き換えてしまうパターン。
ユーザーの入力した文字列を置いておくとは知らずに誤って書き換えてしまい、期待とは異なる結果が返ってきてしまうバグが生まれかねない。

次にありそうなパターンとしてはflagが入力モードを制御しているとは分からずに入れてしまう可能性。
現状は最初のモード分岐のif文さえ乗り越えられればflag変数は使用されていないためそこまで不都合ないかもしれないが、後々に色々な使われ方をしてしまいいつか崩壊する危険性が潜んでいると言える。

他にも考えうることは多くあるが、それらのバグを防ぐためにプログラマに優しい変数名を命名することが重要である。最近の言語はIDEを使用することで定義した変数名を予測で補完してくれたりするので長い名称になってもそこまでプログラミングの手間は増えないだろう。手間がかかってもバグの発生が少なくなるならその分のコストを払うべきである。

変数名のルール

追記予定…

編集後ソースコード

とりあえずはsenをinputCharactors, outをoutputCharactor, flagをmodeFlagとした。

#include <stdio.h>

int main(void) {
    char inputCharactors[1000];
    char outputCharactor;
    int i = 0;
    int modeFlag;
    
    while(1){
        printf("暗号作成の時は0を\n暗号解読のときは1を\n終了するときは2を入力してください:");
        scanf("%d",&modeFlag);
    
        if (modeFlag == 0 || modeFlag == 1) {
            if (modeFlag == 0)
                printf("暗号化させたい文を入力してください\n");
            else
                printf("解読させたい文を入力してください\n");

            scanf("%s",inputCharactors);
        
            while (inputCharactors[i] != NULL) {
                if (inputCharactors[i] >= 65 && inputCharactors[i] <= 77)
                    outputCharactor = inputCharactors[i] + 13;
                else if (inputCharactors[i] >= 78 && inputCharactors[i] <= 90)
                    outputCharactor = inputCharactors[i] - 13;
                else if (inputCharactors[i] >= 97 && inputCharactors[i] <= 109)
                    outputCharactor = inputCharactors[i] + 13;
                else if (inputCharactors[i] >= 110 && inputCharactors[i] <= 122)
                    outputCharactor = inputCharactors[i] - 13;
                else outputCharactor = inputCharactors[i];
                printf("%c",outputCharactors);
                i++;
            }
            i = 0;
        }
        else if (modeFlag == 2)
            return 0;
        else
            printf("正しい数字を入力してください(怒)");

        printf("\n");
    }
}

変数名はわかりやすい名前をつけよう。

今後もこのソースコードを用いてリファクタリングしていく。