新人が教わったリファクタリング諸々

新人エンジニアです。
C#を学習するようになってから初めてリファクタリングを経験し、なんとか消化するためサンプルとして書いていただいたコードを絶賛写経中です。自分で見返す用に、リファクタリング時に教わったことをまとめておきます。エディタはVisualStudio2015。

リファクタリングとは

コード実行時の振る舞いを変えずに、内部構造を改善すること。内容としてはコードの可読性を上げたり、修正が簡単になるように書き換えたりする。
これをいつやるかというと「同じ処理を3度やっていると気づいたらリファクタリングする」というのがひとつの目安というか定石らしい。デバッグ時や機能追加時にもやるみたい。

参考はこちら → リファクタリングとは何か？
以下、具体的に行ったこと。

リファクタリング前のコード

「データベースから人のIDと名前を読み込み、フォームに反映する」という処理を行うものとする。

私が書いてたのがこういうコード。

 private ExampleDatabase modeldata

 //データベースから読み込み
 private void readdata(){
  modeldata = hogentity.ExampleDatabase.FirstOrDefault(test => test.ID == ID.Text)
  Firstname.Text = modeldata.Firstname;
  Lastname.Text = modeldata.Lastname;
 }

 //フォームを初期化
 private void clearForm(){
  modeldata = null;
  ID.Text = "";
  Firstname.Text = "";
  Lastname.Text = "";
 }

で、これから
・modeldataの値が変動したときにTextboxに適切な値が自動的に入るようにする
・clearFormでの処理はmodeldataがデータベースの型で新しいインスタンスを作成するようにする
という処理に書き換えたい。

コードを書き換える

先に「modeldataの値が変動したときにTextboxに適切な値が自動で入るようにする」というのをやる。modeldataの値が変動するのはreaddataもしくはclearFormを呼び出したとき。

readdataは検索ボタンのClick時とか、主キーが入るテキストボックスのValueChangedとかで呼び出し、clearFormのほうは初期化ボタンClickなどで呼び出すという想定。

まずは

private ExampleDatabase modeldata

これを

private ExampleDatabase _modeldata;
private ExampleDatabase modeldata {
    get { return this._modeldata; } //modeldataが参照されたときに返す値
    set { //data変数に値が代入されたときに実行する処理
         _modeldata = value; //通常の代入を行う処理
         ID.Text = modeldata.ID;
         Firstname.Text = modeldata.Firstname;
         Lastname.Text = modeldata.Lastname;
        }

こうする。

正直プロパティに関しての理解度がイマイチなので、なんぞやってなったとき用にコメント残しておいてる。

で、こうするとmodeldataの値が変化したときに、setプロパティが呼ばれて{}内の処理を行う。今回の場合はテキストボックスにmodeldataの値がセットされる。そしてreaddataとclearFormメソッドのほうは

 //データベースから読み込み
 private void readdata(){
  modeldata = hogentity.ExampleDatabase.FirstOrDefault(test => test.ID == ID.Text)
 }

 //フォームを初期化
 private void clearForm(){
  modeldata = new ExampleDatabase();
}

でおｋということになる。
最終的にこう。

private ExampleDatabase _modeldata;
private ExampleDatabase modeldata {
    get { return this._modeldata; } 
    set {
         _modeldata = value; 
         ID.Text = modeldata.ID;
         Firstname.Text = modeldata.Firstname;
         Lastname.Text = modeldata.Lastname;
        }

 //データベースから読み込み
 private void readdata(){
  modeldata = hogentity.ExampleDatabase.FirstOrDefault(test => test.ID == ID.Text)
 }

 //フォームを初期化
 private void clearForm(){
  modeldata = new ExampleDatabase();
}

読み直し時にmodeldataがnullだった場合とかは割愛。
これでのちのち項目が増えてもsetプロパティ内だけ直せばおｋという感じになった。

以下はコードを見てもらってほかに教わったことと、ひと工夫のメモ。

#便利なenum
列挙型。数値と文字列のデータを[数値]番目の[文字列]という形式で保有できる。Tostringで文字列が取り出せる。項目が増えることのないものに向いてて、切り替えが簡単にできる。

public enum dataStatus { New, Edit, Copy }
private dataStatus _currentStatus;
        private dataStatus currentStatus {
            get { return this._currentStatus; }
            set {
                this._currentStatus = value;
                StateLabel.Text = this._currentStatus.ToString();

と書いておくとdataStatus.を記入したときに、その後に続く文字として__New,Edit,Copy__だけが抽出できる。VSの補完機能と相まって便利。

さきほどのclearFormを使って、Formの初期化に伴いステータスバーのラベルに「New」を表示させようとすると以下のようになる。

private void clearForm(){
 modeldata = new ExampleDatabase();
 currentStatus = dataStatus.New;

参考 → Microsoft とことんC#

同系統のクラスは固めて配置する

と、参照しやすいゾ。系統っていうのはプロパティとかイベントとか。

#regionと#endregionで囲んだコードは畳める

「やってる処理の内容は単純だけどコードが長い」っていうのが複数のクラスを跨いであるときはregionとしてひとまとめにして畳んだほうがスッキリする。という理由でも同系統のクラスは固めて置いておくのが吉。

///で、ドキュメンテーションコメントを書ける

//か/*～の通常コメントしか知らなかったけど、///を使うとクラスやメソッドに関する説明をXMLファイルとして書き出してくれるという便利な機能。

/// <summary>
/// データを更新する処理
/// </summary> 
private void reload(){
  modeldata = DBdata
}

と書いておく。するとreloadを呼び出したときにマウスオーバーで「データを更新する処理」が表示される。そのほか、どのイベントで適用されるかなどの情報も追記できる。

if文の層は深くならないように書く

if文の中で行う処理は簡潔に済ませる。
たとえば「dataがnullじゃなければ保存などの処理を進める、nullならフォームに戻る」という処理を書く場合

if(this.data != null) {
  this.save;
  ・
  ・
  ・
} else {
  return;
}

この書き方よりも

if(this.data = null){
  return;
}

  this.save;
  ・
  ・
  ・

という書き方が好ましい。分岐して処理させる必要なく、場合により文章等を書き分けたいという場合であれば三項演算子で書いてもいい。

でも三項演算子読みにくいなーと思ってたのでこちらを参照しました。
→ 条件演算子（三項演算子）を可読性低いとか言わせない

string message = (data = null 
                ? "必須入力項目が空欄です。書き直してください" 
                : "良さげ" );

改行を加えると可読性が上がる説。

(bool型変数)はtrueとして扱う

真偽値を判定する際、()の中に入れたbool型はtrueとして扱う。
そのため

bool ABC = true;
//1
if(this.ABC == true){
  TextBox1.Text="OK";
}

//2
if(this.ABC){
  TextBox1.Text="OK";
}

1、2の処理はどちらもtrueの判定となりTextBox1.Text="OK"の処理が実行される。
逆に、falseであるかを判定したい場合は(!this.ABC)と書く。

以上。そのほか思い出したこととか細細書きたいことがあれば追記予定。