会社でやってるクソみたいなC言語

はじめに
OITの皆さん、お疲れ様です。AdventCalendarなるものを知人に勧められたので12月13日分「会社でやってるクソみたいなC言語」について綴っていきます。表題の通り、今日は私が入社してから今まで遭遇してきた、クソみたいなソースコードを紹介していこうと思います。

断言しておきますが、このプログラミングが進んだ世の中で、社会に出て「人のコードを見ない」なんてことはまずありません。そして人のコードを見るということは自分のコードも見られることになります。

今日紹介するのはC言語のしかも初歩の初歩レベルのことばかりかもしれません。しかし基本的な部分は他の言語でも応用が効きますし、人に見られることを忘れていると基本すらもしなくなりクソースコードを積み上げることになるんです。社会に出てしばきまわされることのないよう、読みやすいコードを書くということを覚えていきましょう。

なお、「if(hoge){}の改行の位置」や「case 処理 break;を一行でまとめるか」、「defineとstatic const」など、派閥にまつわるものについては割愛します。

自己紹介
底辺IT社畜の女の子です。以上。

１．コピペ祭り
始めのお題はコレです。当たり前だよね。ん？
言うまでもありませんがコピペはいけません。ですが会社でもコピペってされてました。なんでやねん。同じようなオブジェクト生成処理なのに基底クラスとかできないからってコピペばっかり。どこが固有処理なのかもわかんないぞこら。こんなのが一ファイル1万行に垂れ流されてるの見た日にゃもうやけ酒一直線じゃないですか。

そんな訳で、同じ処理が並ぶ自体、コードが無駄に大きくなるため、百害あって一利なしです。こういうときは関数化をきっちりしましょう。共通して使いそうな処理を機能ごとに分割し、関数化しておけば、再利用の助けになります。また、関数化によって処理に名前を付けることによって、コードを見る人がどういった意図でソースを書いたのか把握しやすくなります。

必ずやりましょう。必ず。

２．マジックナンバーの大嵐。
ある朝こんなコードを見ました。

       return GetWorldPosX(5) * 3.1; //実際はX座標を取るなんて生易しいものではなかったですが、
                                     //社費義務のため、とりあえず汎用的なもので。

なんじゃこれは。5ってどっから来るんじゃ。3.1ってなんや。πか？ ※5はリソース内のインデックス番号でした。
このように、一般的な数式でもないオリジナルコードでは、数字一つでも意味を図りかねます。会社ではこの5がそこらじゅうに並んでいたため、処理変えようにもあっち行って変えてこっちきてまた変えて・・・。ｱｰｷﾚｿ

このような数字はマジックナンバーと呼ばれ、可読性を著しく下げます。変数やdefineを使いましょう。インデックス番号ならINDEX_NOとか。変数の意味ってめちゃくちゃ重要なんです。講義ではaとかbとか使ってたと思いますが、あとで見る人のことを考えて、ちゃんと名前を考えてあげましょう。

３．インデント祭り

 bool hage, hige, huge, hege, hoge;
            if (hage)
            {
                if (hige)
                {
                    if (huge)
                    {
                        if (hege)
                        {
                            if (hoge)
                            {
　　　　　　　　　　　　　　　　//クッソ長い処理
                            }
                        }
                    }
                }
            }

なんだその例外だらけの処理は。お前はアレか？右寄せで書かないと文字が書けないのか？お前は本当に日本人なのか？外注の中国人じゃないだろうな？

このように、if文などでインデントが右に右に寄りまくったコードは見にくくて半端ではありません。醜いです。

場合にもよりますが、この場合、一番効果的なのが早期リターンです。

if(hoge){ 膨大な処理～～ } でmain文を巨大化するより、
if(!hoge)return; で例外を弾いてしまえば、条件式に気を取られずにソースを読み進められます。
elseの場合の処理も巨大なのであれば関数化してしまいましょう。

bool hage, hige, huge, hege, hoge;
if (!hage) return;
if (!hige) return;
if (!huge) return;
if (!hege) return;
if (!hoge) return;

//この下に処理を書いていく

こんな感じで。処理がながければ長いほどありがたくなるんです。いやほんとに。

４．switch文の中にswitch文

int a = 1;
int b = 2;
switch (a)
{
    case 1:
        switch (b)
        {
            case 1:
                break;
            case 2:
                break;
            case 3:
                break;
            case 4:
                break;
            //ケースがずらーっと
        }
        break;
    case 2:
        break;
    case 3:
        break;
    case 4:
        break;
    //ケースがずらーっと
}

なんでそんなに分岐を一箇所に集めて並べ奉りたいんだ。殺すぞ。
switch文はif else if elseといった場合分けの処理を綺麗にまとめてくれますが、スレッドの進みを把握しにくくなります。case50個の中にcase50個なんて書かれててどこにどういくかなんてわかると思いますか？わからんわ。でも会社でやってた。信じらんない。
caseの中でcase処理をしてる時点で二つのcaseには関連性がほとんどありません。この場合、case分けをしてる部分を関数化。

void switchB(int b)
{
    switch (b)
    {
        case 1:       /*処理*/          break;
        case 2:       /*処理*/          break;
        case 3:       /*処理*/          break;
        case 4:       /*処理*/          break;
            //ケースがずらーっと
    }
}
void switchA(int a, int b)
{
    switch (a)
    {
        case 1:                    switchB(b);                break;
        case 2:                    /*処理*/                   break;
        case 3:                    /*処理*/                   break;
        case 4:                    /*処理*/                   break;
            //ケースがずらーっと
    }
}

これで一気に見やすくなります。またcase条件は大抵整数なので、enumやdefineでcaseに名前をつけてあげましょう。これでどういった条件でこのcaseにいってるのかひと目でわかるようになります。ゲーム関係でステートとか使う際は必ずやりましょう。

ただcaseが50個や100個・・・となるとswitch文だけで馬鹿でかくなるので、関数ポインタやC++以降のデリゲートとか使って条件ごとに振る舞いかえてやると短くて条件追加が楽になります。下地組みが大変ですが。1個のswitchでcaseが10個程度ならswitchで良いと思います。C言語でオブジェクト指向とかかます際はやってみてはいかがでしょうか。関数ポインタ。

６．関数名に１，２

int set_pos1(int a) //一例です。
{
    
}
int set_pos2(int a, int b)
{
   
}
int set_pos3(int a, int b, int c)
{

}

何のための関数化なんや。こんなんで処理分けても意味無いやないかい。しばくぞおい。
関数はその処理そのものを表す大事な記号です。aやbなんて関数あっても誰も使わないでしょう？わたしも使いたくありません。でも使わざるを得ませんでした・・・。こんなふざけた名前の関数が恐ろしいほどのファイルで使われてましたので、変えるわけにも行かなかったんです・・・。絶望やぞ。
個人的に関数の名前のルールとしては、まず何をするかの動詞、そして目的語、詳細と続けていけば良いのではないでしょうか。座標を代入するなら

void SetPos(const vector3* pos);
void SetPosAng(const vector3* pos, const vector3* ang);

もしくは

void set_pos(const vector3* pos);
void set_posData(const vector3* pos, const vector3* Data);

こんな感じでしょうか。これならプロト宣言だけで名前と引数から大体処理の予想がつきます。更に名前が長くなってきてると気づいたら構造化してしまおうという考えにも至ります。あとから使う人の負担を減らせるのでやってください。ホントに。

７．コメント
最後に紹介するのはコメントです。一番簡単なんですけど、実は関数化よりも、今日書いた中で一番必要です。
ヘッダーの上部には「誰が、いつ作ったか」、関数の上には「どういった処理なのか、何が返るのか」、処理の横には「どういったことをしているのか、しようとしているのか」くらいは書きましょう。

//========================================================
//  ○○オブジェクトクラス
//
//  date    :   2016/12/11  
//  name    :   sky
//========================================================

//～～を行うクラス
class classA{

 public:
     int getA() { return a; }   //変数Aのゲッター

 private:
        int a;    //変数A（用途）    

};

2012以降のvisual studioではカーソル合わせるだけでコメント表示される機能があるので、これが中々馬鹿になりません。
面倒だとは思いますが、コードを見て、使ってくれる誰かのために、コメントは必ず書いておきましょう。頼むよホント。

おわりに

本当はもっと会社の愚痴を垂れ流したいところですが、これはそういう記事ではないため、ここまでにしておきます。
大体関数化のことしか書いてませんね・・・。それぐらい大事でやっておけってことです。OITでは一人でこなしていく課題は多けれど、多人数でプログラム組んでいくなんてことは早々無いだろうと思います。社会に出てプログラム組んでいくための訓練として、こういう所に気をつけてみてはいかがでしょうか。

以上です。ここまで見ていただき、ありがとうございました。