コードを短く書く努力をするよ(2)

投稿日: 2011/03/28 作成者: masuda

コードを短く書く努力をすると、テストが省ける | Moonmile Solutions Blog
http://www.moonmile.net/blog/archives/2135/

の続きです。上記では refactoring 前を書いていないので、メリットが良く分からない。ということで、冗長な書き方をさらしておきます。

■ステータスバーを設定（冗長パターン）

ボタンなどのコントロールの、MouseEnter と MouseLeave にちまちま書きます。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
''' <summary>
''' Button1 の MouseEnter
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub Button1_MouseEnter(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.MouseEnter
    ' ステータスバーにメッセージを表示
    ToolStripStatusLabel1.Text = "一番最初のボタンです"
End Sub
 
''' <summary>
''' Button1 の MouseLeave
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub Button1_MouseLeave(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.MouseLeave
    ' ステータスバーのメッセージをクリア
    ToolStripStatusLabel1.Text = ""
End Sub
 
''' <summary>
''' Button2 の MouseEnter
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub Button2_MouseEnter(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.MouseEnter
    ' ステータスバーにメッセージを表示
    ToolStripStatusLabel1.Text = "真ん中のボタンです"
End Sub
 
''' <summary>
''' Button2 の MouseLeave
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub Button2_MouseLeave(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.MouseLeave
    ' ステータスバーのメッセージをクリア
    ToolStripStatusLabel1.Text = ""
End Sub
 
''' <summary>
''' Button3 の MouseEnter
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub Button3_MouseEnter(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button3.MouseEnter
    ' ステータスバーにメッセージを表示
    ToolStripStatusLabel1.Text = "一番下のボタンです"
End Sub
 
''' <summary>
''' Button3 の MouseLeave
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub Button3_MouseLeave(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button3.MouseLeave
    ' ステータスバーのメッセージをクリア
    ToolStripStatusLabel1.Text = ""
End Sub

長い…長すぎますッ!!! Visual Studio を使うとヘッダのコメントは自動生成してくれるのですが、これで 3 つしかボタンに割り当てていません。
コード的には、MouseEnter と MouseLeave のワンセットで 20 行かかります。
なので、10 個のボタンがあれば、200 行掛かるわけですね。

■ステータスバーを設定（イベントを共通化、でも冗長）

これでは、あまりにも、なのでイベントを共通化します。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
''' <summary>
''' Button1 の MouseEnter
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub ButtonCom_MouseEnter(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles _
    Button1.MouseEnter, _
    Button2.MouseEnter, _
    Button3.MouseEnter
    ' ステータスバーにメッセージを表示
    Dim msg As String = ""
    If sender Is Button1 Then
        msg = "一番最初のボタンです"
    ElseIf sender Is Button2 Then
        msg = "真ん中のボタンです"
    ElseIf sender Is Button3 Then
        msg = "一番下のボタンです"
    End If
    ToolStripStatusLabel1.Text = msg
End Sub
 
''' <summary>
''' Button1 の MouseLeave
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub ButtonCom_MouseLeave(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles _
    Button1.MouseLeave, _
    Button2.MouseLeave, _
    Button3.MouseLeave
    ' ステータスバーのメッセージをクリア
    ToolStripStatusLabel1.Text = ""
End Sub

おお、ずいぶんすっきりしましたね。イベントをひとつにまとめたので【共通化】されています。elseif の羅列のかわりに select case を使う方法もあります。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
''' <summary>
''' Button1 の MouseEnter
''' </summary>
''' <param name="sender"></param>
''' <param name="e"></param>
''' <remarks></remarks>
Private Sub ButtonCom2_MouseEnter(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles _
    Button1.MouseEnter, _
    Button2.MouseEnter, _
    Button3.MouseEnter
    ' ステータスバーにメッセージを表示
    Dim msg As String = ""
    Dim cont As Control = TryCast(sender, Control)
    Select Case cont.Name
        Case "Button1"
            msg = "一番最初のボタンです"
        Case "Button2"
            msg = "真ん中のボタンです"
        Case "Button3"
            msg = "一番下のボタンです"
    End Select
    ToolStripStatusLabel1.Text = msg
End Sub

さて、日曜プログラマならば、この程度で許せるのですが、【業務プログラマー】ならば、この時点で２点気づかないと駄目です。

・Button1.MouseLeave 等が、二重に記述されている。
・メッセージが埋め込まれている。

イベントが多くなると、Handles を２箇所修正しないと駄目ですね。VB v2.0 特有ではありますが、継続記号「_」を書く必要があるので、結構面倒です。
また、elseif でも select case でも、所詮は【設定】なのですから、配列/リスト/ディクショナリを使ったほうが、【設定】として際立ちます。

■ステータスメッセージとイベントを設定風に変える

これが、前回書いたときのの【意図】です。
設定として記述するので、SETSTATUS のような関数を使って並べていきます。

1
2
3
4
5
6
7
8
9
Public Sub InitStatusBar()
    SETSTATUS(Button1, "先頭のボタンです")
    SETSTATUS(Button2, "真ん中のボタンです")
    SETSTATUS(Button3, "一番舌のボタンです")
    SETSTATUS(ComboBox1, "項目を選択します")
    SETSTATUS(TextBox1, "名前を入力します")
    SETSTATUS(TextBox2, "年齢を入力します")
    SETSTATUS(TextBox3, "住所を入力します")
End Sub

ここのメッセージを enum なり const string なりを参照しても良いですし、外部ファイルから読み込みをしても構いません。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
    Class STATMSG
        Public cont As Control
        Public msg As String
    End Class
    Private _status As New List(Of STATMSG)
    ''' <summary>
    ''' フォーカス時のメッセージを設定
    ''' </summary>
    ''' <param name="c"></param>
    ''' <param name="msg"></param>
    ''' <remarks></remarks>
    Public Sub SETSTATUS(ByVal c As Control, ByVal msg As String)
        Dim sm As New STATMSG()
        sm.cont = c
        sm.msg = msg
        AddHandler c.MouseEnter, AddressOf COM_MouseEnter   'MouseEnter
        AddHandler c.MouseLeave, AddressOf COM_MouseLeave   'MoueeLeave
        _status.Add(sm)
    End Sub
''' <summary>
''' フォーカス時のメッセージを設定
''' </summary>
Public Sub ChangeFocus(ByVal sender As Object, ByVal b As Boolean)
    If b = False Then
        ToolStripStatusLabel1.Text = ""
    Else
        For Each cm In _status
            If cm.cont Is sender Then
                ToolStripStatusLabel1.Text = cm.msg
            End If
        Next
    End If
End Sub

ここまで共通関数で、以下のイベントを form に追加します

1
2
3
4
5
6
Private Sub COM_MouseEnter(ByVal sender As System.Object, ByVal e As System.EventArgs)
    ChangeFocus(sender, True)
End Sub
Private Sub COM_MouseLeave(ByVal sender As System.Object, ByVal e As System.EventArgs)
    ChangeFocus(sender, False)
End Sub

こうすると、コントロールが増えても InitStatusBar 関数に1行追加するだけで済みます。
また、メッセージをファイル読み込みするときは、InitStatusBar 関数でメッセージをファイルから読み込むように書き換えるだえけでいいのです。

■高速化するためにリファクタリングする

この設定では List を使ったのですが、Dictonary を使ったほうがループがなくなります。これはリファクタリングの範囲なので、インターフェースは変わらないように作ります。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
Private _statusdic As New Dictionary(Of Control, String)
''' <summary>
''' フォーカス時のメッセージを設定
''' </summary>
''' <param name="c"></param>
''' <param name="msg"></param>
''' <remarks></remarks>
Public Sub SETSTATUS(ByVal c As Control, ByVal msg As String)
    _statusdic.Add(c, msg)
    AddHandler c.MouseEnter, AddressOf COM_MouseEnter   'MouseEnter
    AddHandler c.MouseLeave, AddressOf COM_MouseLeave   'MoueeLeave
End Sub
 
Public Sub ChangeFocus(ByVal sender As Object, ByVal b As Boolean)
    If b = False Then
        ToolStripStatusLabel1.Text = ""
    Else
        ToolStripStatusLabel1.Text = _statusdic(CType(sender, Control))
    End If
End Sub

Control から string を引き出すだけなので Dictionary が使えます。
まあ、List をループさせても、コントロール数はそう多くないので(1万個とかではないでしょう)、スピードは大して変わりません。

というわけで、こんな風に行数を減らしていきます。
今回のポイントとしては、

・冗長な部分を共通化させて、行数を減らす。
・設定なのかロジックなのかを区別する。設定ならば、設定風に書き換える。
・リファクリングをして高速化する。

ってのがミソです。

業務コードの場合、単純にコピー＆ペーストで済めば、それを使っても良いのですが、コード自体が冗長になる場合は考え直したほうがよいですね、という話です。このあたりは、保守性も含めると、短いコード（保障されたコードを再利用する）というのが、業務コードを書くときのコツです。

カテゴリー: 開発, 設計 パーマリンク

コードを短く書く努力をするよ(2) への1件のコメント

  1. masuda のコメント:
    2011/03/28 11:30 am

    ちなみに、10画面程度であれば、200 x 10 = 2000 行の無駄はたいしたことはないのです。
    業務的に 100 画面あれば、200 x 100 = 2万行あるわけで、これは誤差にしては、ちょっと、っていう数字なのです。なので「日曜プログラマ」的には、このままでいいけれど、「業務プログラマ」ならば許されない、というお話です。

コメントは停止中です。