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

2025年2月24日2025年3月16日

久々に大学時代に使用していた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 という変数名が使われているが、変数名だけでは何を扱っているのか分かりにくい。

例えば sen は入力された文字列を保持する配列だが、意味が直感的に伝わらない。また、flag も動作モードを表しているが、用途が分かりにくい。

こうした曖昧な変数名が増えると、以下のような問題が発生する可能性がある。

❌ バグの発生リスクが高まる

sen の中身を誤って書き換えてしまう → 意図しない出力になる
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");
    }
}

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

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