【開発】自身のコーディングの癖について考えてみる

2019/05/032019/08/30DEVELOP

はじめに

綺麗なプログラムコードを書く人ってどんな言語であろうと綺麗に書きますよね。少し気になって、自分自身のコーディングの癖を調べてみました。そうすると、僕自身にも癖がありましたので紹介したいと思います。また、プログラム言語はC#となっています。

インデントは揃える

if(a){
    if(b){
        // 何か
        if(c){
            // 処理を
            if(d){
                // 挟む
            }
        }
        else if(e){
        }
        else{
        }
    }
}

こんな風に、ネストが深くなると読みにくくなるので揃えます。

if(a)
{
    if(b)
    {
        // 何か
        if(c)
        {
            // 処理を
            if(d)
            {
                // 挟む
            }
        }
        else if(e)
        {
        }
        else
        {
        }
    }
}

視線を横にずらさなくて良い。上から下に縦にストンッと。
この程度の行数なら問題ないかもしれませんが行数が増えると大変。
好みもありますが僕は結構こちらを使います。
そもそもネストし過ぎはよくないのですけれど、バランスが大事かなって。

スコープを使う

例えばイベント用のパラメータを次々登録する様なDataManagerがあるとして

void SetupDataList()
{
    CData data;
    data.name = "hoge";
    data.param1 = 1234;
    DataManager.Instance.Add(data);
 
    CData data2;
    data2.name = "hoge2";
    data2.param1 = 1234_2;
    data2.param2 = "test";
    DataManager.Instance.Add(data2);
 
    CData data3;
    data3.name = "Oh";
    DataManager.Instance.Add(data3);
}

ここではパラメータを次々と追加、登録していています。ただ、スコープの関係でコンパイルエラーになるので追加するデータのクラス名を変更していますが、なんかかっこわるい。

スコープを使えば…

void SetupDataList()
{
    {
        CData data;
        data.name = "hoge";
        data.param1 = 1234;
        DataManager.Instance.Add(data);
    }
    {
        CData data;
        data.name = "hoge2";
        data.param1 = 1234_2;
        data.param2 = "test";
        DataManager.Instance.Add(data);
    }
    {
        CData data;
        data.name = "Oh";
        DataManager.Instance.Add(data);
    }
}

名前を気にすることなく記述できます。また、変数やクラスの有効範囲が絞られ、スコープの外で参照されないことが確約されるため影響範囲を絞れます。

対になるデータはまとめる

List<Unit> DisplayRareUnitList;                 // ユニット
List<AnimationBase> PickupUnitAnimationList;    // ユニットアニメーション
int index = 0;
DisplayRareUnitList[index].Setup();
PickupUnitAnimationList[index].Setup();

対になっている様なデータはひとつのデータクラスにまとめた方が、
表示とデータのずれが発生する可能性が回避できたりと、
コード的にも目にも優しいです。

public class DisplayUnitInfo
{
    public Unit DisplayRareUnit;
    public AnimationBase PickupUnitAnimation;
};
private List<DisplayUnitInfo> displayUnitInfoList;

int index = 0;
DisplayUnitInfo info = DisplayRareUnitList[index];
info.DisplayRareUnit.Setup();
info.PickupUnitAnimation.Setup();

ラムダ式の誤った使い方

便利ですよね、ラムダ式

gameObject.Click += (sender, e) => { fugafuga(); };

これとかよくやると思います。ただ、誤った使い方をすると
いわゆるスパゲッティコード一直線になるので注意です。
まず、以下のコードを見て下さい。

CreateDialog ("メッセージ", (Data _data) => { _data.update() }, null, () => {});

さて、CreateDialogはダイアログを作っています。
引数部分にいろいろ書かれていますけど、何をしているかお分かりでしょうか？

ざっと説明しますと

一つ目がダイアログのOKボタンを押した時
二つ目がダイアログのCANCELボタンを押した時
三つ目がボタンを押した後に閉じるアニメーションが完了した時
にそれぞれ呼ばれる様になっています。

一目で分かりません。メソッドの中に潜ってはじめて分かる様なものにラムダ式を書くのは避けましょう。上記の場合は面倒臭がらず関数をよいして引数として渡してあげましょう。

CreateDialog ("メッセージ", OnOk, OnCancel, OnCloseAnimationFinished);

そもそもラムダ式はインラインコードを簡潔に書くためのもの(と記憶している)ですし、誰が見ても分かる様にイベントハンドラには名前をつけた方が良いと思っています。

APIは薄くラップしておく

AES.Encrypt(_plainText );    // 暗号化
AES.Decrypt( _cipherText );  // 復号化
// ラップしておく
public string AppDecrypt( string _cipherText )
{
    return AES.Decrypt( _cipherText );
}
public string AppEncrypt( string _plainText )
{
    return AES.AppEncrypt(_plainText);
}

薄くラップしておけば、後で一括変更しやすいです。
これはフレームワークを作る時によくやるやつです。

実際、スマホアプリのStore申請直前まで

public string AppDecrypt( string _text )
{
    if(IsEncrypt())
    {
        return AES.Decrypt( _text );
    }
    return _text;
}
public string AppEncrypt( string _text )
{
    if(IsDecrypt())
    {
        return AES.AppEncrypt(_text);
    }
    return _text;
}

の様な実装をしていましたが、過去に「やっぱり暗号化するかしないかの判定は外して欲しい」と変更指示がありましたが、IsEncrypt、IsDecryptの中身を必ずtrue を返すだけの対応で済みました。これが直接APIを叩く作りにしていたらと思うと面倒ですよね。小さなことなんですが、効率的なプログラムってこんなところから出来ていくものなのかなって思っています。