かってにコードいじって自己満足に浸る(2)

C#

手元に下記のようなメソッドがある。
身バレ防止でだいぶ簡略にしてあるが実物はもっと改装が深くて条件が複雑である。

        public void DoAnything()
        {
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    DoWrite();
                }
            }
        }

別に動作には問題ないのだが個人的にはDoWrite()を実行する条件をメソッドに切り出しておきたいなあ、と思うものである。
従って動作を変えないように注意深く変更していく。

１　まずはフラグ(flg)を用意

            bool flg = false; //フラグ
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    DoWrite();
                }
            }

２　DoWrite()直前でフラグを立てる

            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;//フラグが立ったよ
                    DoWrite();
                }
            }

３　DoWrite()をif(flg){ }で囲う

            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                    if (flg) //囲う
                    {
                        DoWrite();
                    }
                }
            }

４　if(flg)ブロックを一番外のifブロックの外に追い出す

            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
            }
            if (flg) //追い出されました
            {
                DoWrite();
            }

５　flgの宣言～flgの値を決定している部分をVisualStudioの"メソッドの抽出"

            bool flg = NewMethod(); //メソッドにされてしもた
            if (flg)
            {
                DoWrite();
            }

        private bool NewMethod() //メソッドになりました
        {
            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
            }

            return flg;
        }

６　bool flg = NewMethod();のflgをVisualStudioの"インラインの一時変数"（これVS2010にはないんだよなぁ）

            if (NewMethod())
            {
                DoWrite();
            }

７　完成なう

        public void DoAnything()
        {
            if (NewMethod())
            {
                DoWrite();
            }
        }

        private bool NewMethod()
        {
            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
            }

            return flg;
        }


やってみての感想
・条件が変わったときにNewMethod()の中身だけを変更すればいいことが明確になる。

自己満足完了

かってにコードいじって自己満足に浸る

C#

手元に下記のようなメソッドがある。
末尾returnを強制されてた人が書いたと思われる。
身バレ防止でだいぶ簡略にしてあるが実物はもっと改装が深くて条件が複雑である。

        public bool IsSuccess()
        {
            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
            }
            return flg;
        }

別に動作には問題ないのだが個人的にはfalseになるのわかってるならさっさと脱出してほしいなあ、と思うものである。
従って動作を変えないように注意深く変更していく。

１－①　一番外側のif文にelse節をくっつける。

            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
            }
            else //これつけた
            {

            }
            return flg;

１－②　return flg;を一番外側のif節およびelse節の末尾に分配

            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
                return flg; //これつけた
            }
            else
            {
                return flg; //これつけた
            }
            //これ消した return flg;

１－③　else節はfalseしか返さないのでreturn false;と書き換える

            bool flg = false;
            if (value1 > 50)
            {
                if (value2 < 30)
                {
                    flg = true;
                }
                return flg;
            }
            else
            {
                return false;//書き換えた
            }


１－④　一番最初のif文の条件を反転させてif節とelse節の中身を入れ替える

            bool flg = false;
            if (!(value1 > 50)) //条件反転
            {//中身入れ替え
                return flg;
            }
            else
            {//中身入れ替え
                if (value2 < 30)
                {
                    flg = true;
                }
                return flg;
            }

１－⑤　else{ }を除去　（{}の中は消さない）　※if{}の末尾でreturnしているのでelse{ }があってもなくても同じことである

            bool flg = false;
            if (!(value1 > 50))
            {
                return false;
            }
            if (value2 < 30)
            {
                flg = true;
            }
            return flg;

2番目のif文についても1番目のif文と同じ作業を実施
　２－①　else節をつける
　２－②　return flg;をif節およびelse節の末尾に分配
　２－③　else節はfalseしか返さないのでreturn false;と書き換える
　２－④　条件を反転させてif節とelse節の中身を入れ替える
　２－⑤　else{ }を除去　（{}の中は消さない）

            bool flg = false;
            if (!(value1 > 50))
            {
                return false;
            }
            if (!(value2 < 30))
            {
                return false;
            }
            flg = true;
            return flg;

２－⑥　最後のreturnはtrueしか返さないのでreturn true;に書き換える。
２－⑦　出番のなくなったflgを除去

            if (!(value1 > 50))
            {
                return false;
            }
            if (!(value2 < 30))
            {
                return false;
            }
            return true;

２－⑧　!(cond)を整理する
２－⑨　if文の中かっこを除去する

            if (value1 <= 50)
                return false;

            if (value2 >= 30)
                return false;

            return true;


やってみての感想
・"条件を反転させてif節とelse節の中身を入れ替える"と"!(cond)を整理する"は若干怖い作業だなと思う。誰か自動化してくれ。
・コードの動きは動画にしたほうが理解されやすいような気がする。
・条件をすっきりさせられた気がする。

自己満足完了

桜花賞なう

今日近所の阪神競馬場行ったんです。

そしたらなんか人がめちゃくちゃいっぱいでラチ沿いに行けないんです。

で、よくみたら桜花賞、とか書いてあるんです。

もうね、アホかと。馬鹿かと。お前らな、３歳牝馬限定ＯＰ芝マイル如きで普段来てない阪神競馬場来てんじゃねーよ、ｳﾞｫｹが。
なんかカップルとかもいるし。二人で仁川かよ。うらやましいな。
よーしオレ武豊の単勝買っちゃうぞー、とか言ってるの。もう見てられない。
おまえらな、ペグシルやるからそこ退け。
競馬場ってのはな、もっと殺伐としてるべきなんだよ。
はずれ馬券破ってばらまく奴といつ喧嘩が始まってもおかしくない、
万券取るかお馬で人生アウトか、そんな雰囲気がいいんじゃねーか。素人はすっこんでろ。

 

・・・・とは言いません。本日は多くの皆様に阪神競馬場にご来場ありがとうございますm(_ _)m。ついでに私の配当のために馬券をお買い上げいただきありがとうございます。皆様のはずれが私のあたりです。よろしくおながいいたします。

 

なんちって。

ヌルポとみせかけてヌルリ

C#

ヌルポってC#ではヌルリ（NullReferenceException）なんだね。知ってても何の得もないけど。
それはともかく、ヌルｐ・・・もといヌルリで落ちるのは恥とでも思われているのか、やたらといたるところでNullチェックを強いられているコードが目の前にある。

    static class Checkers
    {
        public static bool IsNull(object p)
        {
            return p == null;
        }
    }

こんなのがあって、いたるところでCheckers.IsNull(Hoge)みたく使うことを強いられている。
それはまあいいのだが、このCheckers.IsNull(Hoge)の使い方でおやおや？とおもったのがint型のプロパティまでNullチェックしてること（↓コード）。
羹に凝りて膾を吹く、を地でいくような世界。

        private static void Proc2()
        {
            myclass m = new myclass();
            bool result2 = Checkers.IsNull(m.NonNullableValue);     //これはつねにfalse
            Console.WriteLine("{0}", result2);
        }

    class myclass
    {
        public int NonNullableValue { get; set; }
    }

まさかint型プロパティの持ち主のNullチェックもできるのか？とおもったが

        private static void Proc3()
        {
            myclass m2 = null;
            bool result2 = Checkers.IsNull(m2.NonNullableValue);    //ヌルリ
            Console.WriteLine("{0}", result2);
        }

ヌルリで落ちた。うーん、int型プロパティまでNullチェックしようとした人の気持ちがわからん。チェックに引っかかったケースの動作確認どうしたのだろう？

テストのお勉強。

C#


Nanka.Hoge()の戻り値が10であることをテストする際の

Assert.AreEqual(10,Nanka.Hoge());

て書き方に慣れない。「10がNanka.Hoge()と等価である」と読んでしまう。
できうることなら引数を逆にして

Assert.AreEqual(Nanka.Hoge(),10);

と書きたいが戻り値が9だった場合、つまりテスト失敗したときのメッセージが
「<9> が必要ですが、<10> が指定されています。」
となってしまい、<10>のほうが間違ってるように読めてしまうのでまずい。

え？テスト失敗させなきゃいいんじゃんって？

んなあほな。

 

関係ないけどはちロケかわいいよね。あんな娘がいればもう・・・心配だな。

 

 

追記

なんで慣れないのかというと、if文では

if(Nanka.Hoge()==10)て書くじゃん。

if(10==Nanka.Hoge())て書いたらおかしいじゃん。

 

 

関係ないけどはちロケかｗ・・・もういいか。

 

 

 

どうにもいらつく（やりなおし）

ソースコード部分見やすくするためにはてな記法を使うことにした。


お仕事で見ているコード。言語はC#

class Person
{
 public string Address { get; set; }
 public string Name { get; set; }
 public int Age { get; set; }
 public string Profession { get; set; }
 public string GetCSV()
 {
  const string comma = ",";
  string result;
  result = Address + comma;
  result += Name + comma;
  result += Age.ToString() + comma;
  result += Profession;
  return result;
 }
}


GetCSV()は自分自身の内容をCSV形式で吐き出すメソッド。
もちろんちゃんと動く。動くのだが読んでいてイラつく。
①最初の項目(Address)とそれ以外の項目でresultへの代入が違う(=,+=)
②最後の項目(Profession)とそれ以外の項目で末尾のcommaの有無が違う
③commaの登場回数が多い。constにしてるだけマシだが。

最初に書いた人、なんでこんなコードにしたんだろう？


＜追記したんご＞
↑のコードはVB6で書かれていたのをC#に移植したものらしいのだが、VB6のコードを確認したところ、
①各項目をCollectionにAdd
②Collectionを配列に変換
③Join()で連結
というまっとうな方法になっていた。
なんで後発言語のC#のほうがへんなコードになったんだろう？

＜さらに追記＞
こんなこと気にしててはいけないのだろうか。だめなのだろうか？だめといわれても治らないでしょうが。

どうにもいらつく

お仕事で見ているコード。言語はC#

 

class Person
{
public string Address { get; set; }
public string Name { get; set; }
public int Age { get; set; }
public string Profession { get; set; }

public string GetCSV()
{
const string comma = ",";
string result;
result = Address + comma;
result += Name + comma;
result += Age.ToString() + comma;
result += Profession;
return result;
}
}

 

GetCSV()は自分自身の内容をCSV形式で吐き出すメソッド。

もちろんちゃんと動く。動くのだが読んでいてイラつく。

①最初の項目(Address)とそれ以外の項目でresultへの代入が違う(=,+=)

②最後の項目(Profession)とそれ以外の項目で末尾のcommaの有無が違う

③commaの登場回数が多い。constにしてるだけマシだが。

 

最初に書いた人、なんでこんなコードにしたんだろう？

 

 

＜追記したんご＞

↑のコードはVB6で書かれていたのをC#に移植したものらしいのだが、VB6のコードを確認したところ、

①各項目をCollectionにAdd

②Collectionを配列に変換

③Join()で連結

というまっとうな方法になっていた。

なんで後発言語のC#のほうがへんなコードになったんだろう？

 

＜さらに追記＞

こんなこと気にしててはいけないのだろうか。だめなのだろうか？だめといわれても治らないでしょうが。