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

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

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

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

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

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

    ''' <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 行掛かるわけですね。

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

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

    ''' <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 を使う方法もあります。

    ''' <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 のような関数を使って並べていきます。

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

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

	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 に追加します

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 を使ったほうがループがなくなります。これはリファクタリングの範囲なので、インターフェースは変わらないように作ります。

    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万行あるわけで、これは誤差にしては、ちょっと、っていう数字なのです。なので「日曜プログラマ」的には、このままでいいけれど、「業務プログラマ」ならば許されない、というお話です。

コメントは停止中です。