【C#】リーダブルコード・リベースド

マップエディタの開発後記です。
しばらくはこの関連の記事が続きます。

C#によるWindowsフォームアプリケーションの開発です。
そしてVisual Basic.NETフォームアプリケーションというレガシーコードからの脱却です。
それがこの開発のテーマになっています。

今回の主な内容はリファクタリングになります。
コンテンツの進展はありませんが、どうしてもここでお話したい内容のため、記事にしておこうと思います。大事な事だと思ったので😃

書き直す理由 (改)

前回の記事と索引が被ってるんですが、やっぱりもうちょっと言いたい事があります。
正直なところ昔の自分クソすぎるやろアピールをして潰したいだけな自己満足ネタになってしまい、そんなもの誰が興味あるの？って感じなんですが、どうせこんな記事誰も見ないし、そもそも人間はダメなところを真摯に受け止めてそれを持って前に進むべきだと私は思うので、ダメなものはダメとはっきり言わないといけないと思います。

例えそれで自分自身の心がダメージを受けようとも。
自傷行為…かどうかは知らないですが。

前置きが長くなるのもアレなんでもう本題なのですが、まず今作っているC#.NET製マップエディタの元プログラムであるVB.NET製マップエディタのダメな点について。

これは前回の記事に粗方書いたのでそっちの記事を見てください。

【C#】VB.NETのコードを書き直す (4)

マップエディタの開発後記です。このプロジェクトの今後とプロジェクト指向についての考察。

www.loopunlimited-rootone.com

2023.10.23

そして、その続きなんですけど、
Visual Basic.NET自体の問題についてです。
これについても以下のぺージをまず共有しておこうかなと思います。

くたばれVisual Basic(.net) in 2023

---

・・・結構いっぱい書かれてますね。
うん。はい。

正直うんざりしたというのが感想です。

Visual Basic.NET自体にね( ^ω^)

なにこの言語。
これは使いたくないですね。

VB.NETはC#.NETの劣化版だという前哨の批判（概略？）があります。
これは私も同意せざるを得ない事情です。

そして例えば、私も過去に作ったVB.NETのコードの一部に以下の構文がありました。

<Global.Microsoft.VisualBasic.CompilerServices.DesignerGenerated()>
Partial Class Form1
    Inherits System.Windows.Forms.Form

    Friend WithEvents FlowLayoutPanel1 As FlowLayoutPanel
    Friend WithEvents FlowLayoutPanel2 As FlowLayoutPanel
    Friend WithEvents ChipEnterButton As Button
    Friend WithEvents OpenFileDialog1 As OpenFileDialog
    Friend WithEvents Label1 As Label
    Friend WithEvents Label2 As Label
    Friend WithEvents TableLayoutPanel2 As TableLayoutPanel
    Friend WithEvents TableLayoutPanel1 As TableLayoutPanel
    Friend WithEvents PictureBox1 As PictureBox
    Friend WithEvents Label3 As Label
    '... 省略

この「Friend WithEvents」について、私も最近までよく知らなかったのですが、これはどうやらイベントハンドラをメソッドに紐づける際のHandlesキーワードを使う上で必要な手続きらしいです。

なぜこんな面倒な仕様になっているのか？という事を必然的に思ったあなたは多分PGMerとしてまともではないかと思いますが、これぶっちゃけどのファイルからでもイベントハンドラの登録ができちゃいます。

私が指摘したいのはその部分です。要はPublicアクセスなんですよ。
あれ？カプセル化ってどこに消えた・・・？😑笑

上記のコードはVB.NETのプロジェクトで、Visual Studioのフォームデザイナーからコントロールを追加すると自動的に組み込まれるようです。
それはつまりVB.NETのイベントハンドラ登録は常に公共に公開していきまーす、と言うのと同じ事だと思います。

ちなみにこの指摘は以下のQAサイトでも議論されていました。

.net – ‘Friend WithEvents’ in Visual Basic vs. ‘private’ in C# – Stack Overflow

そういう事も含めて、VB.NETは基本的な言語の方向性として、
「とりあえず意図した通りに動けばいい」
「定義したものは全て実装サイドで適切に制御しないとダメでしょう？」
「VB6（1998年の言語仕様）を守るために、その仕組み（構文法）を踏襲したコード」
というものであるという事だそうです。

そもそも今、VB6なんて2周周回遅れくらいの古いシステムを重んじてる時点で変ですし、その理由も甚だ自分勝手なように思います。

それについても以下のぺージで問題は提起されていて、私はこの記事を見て救いようのない何かを垣間見た気がしました(´・ω・`)

【簡単に解説】VB6からVB.NETへの変換（移行）について

未だにVB6を使ってる企業が多いとかなんとか…。
誰でも簡単に作れるVBコードを作った結果、それが公式からサポートされなくなり、システムが朽ち果てていく様を黙って見つめるだけという実情を抱える方々がいるそうです。
その人たちは一体どう責任取るつもりなのでしょうか。。。
まさかこんな事になるとは思ってなかった、みたいな事を言うんでしょうね。知らないですけど。

以上の事が私が今回「書き直す理由 (改)」で言いたかった内容です。

要するに今の時代、VB.NETでプログラミングをする利益がほとんどない（むしろ可読性が悪すぎて損失的か？）、という事を結論付けざるを得ないでしょう。

あと上記のコードに「FlowLayoutPanel1」や「FlowLayoutPanel2」などの冗長なオブジェクト名が記載されていますが、それは当時の私の脳みそがカスレベルだったからです。

とりあえず動けばいい。
まさにVB.NETの思惑に十中八九嵌っている証拠だと思います。
さきほどのリンクの前回記事でも言ってますが、とりあえず動作するコードを書いていった結果メンテナンスが不可なアナーキー（無秩序）コードになってしまい追加開発が困難になってプロジェクト自体が破綻しています。
これは由々しき問題です。
それを最後に貶す意味でこの記事を残しておこうと思います(笑)

リーダブルコードにする

オブジェクト指向プログラミングの本質とは何か

クラスとか、メソッドとか、オブジェクト指向設計とか、隠蔽とか…

色々な単語で溢れかえって難解と言われるオブジェクト指向プログラミングですが、その根源には「保守性」「可読性」「柔軟性」などがあると思います。

いつ[When]どこで[Where]誰が[Who]何を[What]見てもなぜ[Why]そうなっているのかが分かるコード、それがオブジェクト指向プログラミング（OOP）が目指すべき指標ではないかと思います。

それをどのようにして[How]実装するか、それが今重要視するべき点です。

5W1H*¹にじゃっかん強引に当てはめてみましたが、分かってもらえれば幸いです。

しかしこれらの概念はとても難しい話題です。
単純に重視しようと思っても内容が抽象的すぎて、実践的なプログラミングの現場ではどう当てはまるのか分からないでしょう。

実際私は保守性についても全部分かっているわけではありませんし、可読性においても人によって変わってくるものだと私は思っていて、それなら尚更ベストプラクティスなコードなんて存在しないとさえ思っています。

しかしそれでも10人のうち7人が分かりやすいと思ったならそれはベタープラクティスだと思いますし、万人に納得させるもの、すなわち万人が認めてくれるものを作るべきだと私は考えます。

そんな中でも「可読性」については、プログラミングと直結した部分でメンテナンスやシステムの構造を紐解く部分で重要な点だと私は思っているので、今回はこの部分についてテコ入れをしてみようと思います。

5W1H*¹・・・ビジネスにおける情報伝達指針の一つ。一般的に説明する上で必要な情報を過不足なく伝えるために使われる。

例：描画処理をオブジェクトに依存させる

例えば以下の部分について。

右側の枠にはマップのタイル用の素材が定間隔で配置され、左側の枠に実際のBG背景タイルマップが表示されます。（前回から少しレイアウトを見直しました）

上記の右枠にグラフを読み込む処理はかつて以下のようなコードで書かれていました。

Public Class GraphicChip1
    Public Function Load_ChipList(ByVal imgFile As String, ByVal col As Integer, ByVal row As Integer, Optional ByVal fs As Boolean = False) As Boolean
        Dim panel As TableLayoutPanel = Form1.TableLayoutPanel1
        '... panelのプロパティ変更などをいろいろする 省略
        Form1.TableLayoutPanel1 = panel
        Return True
    End Function
End Class

Public Class FAMESharedClasses1
    Public Class ChipVolume
        Public Shared Property ChipFileName As String
            Get
                Return _chipFileName
            End Get
            Set(value As String)
                _chipFileName = value
            End Set
        End Property

        Public Shared Property X As Integer
            Get
                Return _chipVolumeXY(0)
            End Get
            Set(value As Integer)
                _chipVolumeXY(0) = value
            End Set
        End Property

        Public Shared Property Y As Integer
            Get
                Return _chipVolumeXY(1)
            End Get
            Set(value As Integer)
                _chipVolumeXY(1) = value
            End Set
        End Property
    End Class
    ' ... 他のPublicアクセス(メンバ)変数 省略
End Class

Public Class FileIO1
    Public Function Set_ListOption() As DialogResult
        Dim DialogProvider As ChipLoadDialog
        Dim iShadow As Form1 = New Form1
        Dim ret As DialogResult
        ret = iShadow.OpenFileDialog1.ShowDialog()
        If ret = Windows.Forms.DialogResult.OK Then
            Dim img As Image
            Try
                img = Image.FromFile(iShadow.OpenFileDialog1.FileName)
            Catch
                ' ... 省略
            End Try
            DialogProvider = New ChipLoadDialog(img)
            ' Open the ChipLoadDialog.
            ret = DialogProvider.ShowDialog()
            If ret = Windows.Forms.DialogResult.OK Then
                FAMESharedClasses1.ChipVolume.ChipFileName = iShadow.OpenFileDialog1.FileName
                FAMESharedClasses1.ChipVolume.X = DialogProvider.Get_DialogParameter(1)
                FAMESharedClasses1.ChipVolume.Y = DialogProvider.Get_DialogParameter(0)
            Else
                Return DialogResult.Cancel
            End If
        End If

            Return ret
    End Function
End Class

Public Class ChipLoadDialog
    Public Sub New(ByVal i As Image)
        ' この呼び出しはデザイナーで必要です。
        InitializeComponent()

        ' InitializeComponent() 呼び出しの後で初期化を追加します。
        Descend = i
        ColumnNum.Text = FAMESharedClasses1.ChipVolume.X
        RowNum.Text = FAMESharedClasses1.ChipVolume.Y

    End Sub
    ' ... 省略
End Class

Partial Class Form1
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles ChipEnterButton.Click, マップチップを読み込むToolStripMenuItem.Click
        Dim ret As DialogResult
        Dim iDialog1 As ChipLoadDialog
        Dim iGraphic As GraphicChip1 = New GraphicChip1
        Dim iFileIOs As FileIO1 = New FileIO1

        If ChipEnterButton.Text = "グラフィック取込..." Then
            ret = iFileIOs.Set_ListOption()
            If ret = Windows.Forms.DialogResult.OK Then
                Dim img As Image = Image.FromFile(FAMESharedClasses1.ChipVolume.ChipFileName)
                iDialog1 = New ChipLoadDialog(img)
                iGraphic.Load_ChipList(FAMESharedClasses1.ChipVolume.ChipFileName, FAMESharedClasses1.ChipVolume.X, FAMESharedClasses1.ChipVolume.Y, iDialog1.Get_DialogParameter(2))
            Else  ' DialogResult.Cancel
                Exit Sub
            End If
        Else  ' Button1.Text = "グラフィック削除"
            iGraphic.Clear_ChipList()
        End If
    End Sub
    ' ... 他の処理 省略
End Class

まずForm1のButton1_Clickイベントの中でGraphicChip1やFileIO1などのクラスオブジェクトを生成して、その中のメソッドを呼び出しています。
これは何のために行っているかと言うと、本来行うべき処理を他のファイルに移して完結する処理としてカプセル化するためです。

しかしこのコードには一貫性がなく、GraphicChipというクラス名でありながら、実際に生成しているのはTableLayoutPanelオブジェクトであり、またForm1のコントロールである「TableLayoutPanel1」が、GraphicChip1に依存してしまっています。
これでは、「TableLayoutPanel1」の仕様が変わった際に、Form1とGraphicChip1両方のクラスを修正する必要が出てきてしまい、修正時の影響範囲が大きくなります。

他のFileIO1クラス、ChipLoadDialogクラスも同様です。
更に「FAMESharedClasses1」というクラスの中に多くのPublicプロパティやメンバがあり、このクラスを使ってForm1とそれ以外のクラスとでデータの受渡しをしているようです。

基本的にソースコードは手続き型言語の構造を擁していて、オブジェクト指向言語の構造ではありません。
悪いわけではないし、プログラムに問題があるわけではありませんが、冒頭でも言った「可読性」や「柔軟性」を上げるためにはやはりオブジェクト指向設計にする事は避けては通れません。

---

これをC#に移植します。

まずはグラフィックを表示するTableLayoutPanelコントロールは、Panelコントロールの中に配置されているデザイナー上のコントロールなので、この方式でも悪い事はないのですが、無駄なリソースを使用したくないので、直接Panelコントロールへ配置するように変更します。

namespace ClientForm.src.CustomControls.Chip
{
    public partial class ShowcasePanel : Panel
    {
        // ... ボタンオブジェクトを作成して、定間隔に配置する処理 省略
    }
}

サブクラス（継承）です。

これで作成したPanelオブジェクトをフォームデザイナーで配置していきます。

次にグラフィックデータファイルを選択して、そのファイルをPanelオブジェクトに渡す部分です。
これは、旧エディタではOpenFileDialogで選択したファイルを一時的にローカルメンバへ持たせた後、グラフィックの縦横分割数を決定する独自実装のダイアログを表示していました。

これも管理が煩雑になるので一括管理するべきです。

namespace ClientForm.src.Apps.Loader
{
    public partial class LoadGraphDialog : Form
    {
        public LoadGraphDialog()
        {
            InitializeComponent();
        }

        private void OpenFileDialogButton_Click(object sender, EventArgs e)
        {
            using OpenFileDialog ofDialog = new()
            {
                Filter = "画像ファイル(*jpg;*png;*bmp)|*jpg;*jpeg;*png;*bmp|すべてのファイル(*.*)|*.*",
                Title = "開くファイルを選択",
            };
            if (ofDialog.ShowDialog() == DialogResult.OK)
            {
                filePathTextBox.Text = ofDialog.FileName;
                // ... 画像のバリデーションチェックなど 省略
            }
        }

        private void OKButton_Click(object sender, EventArgs e)
        {
            if (filePathTextBox.Text.Length == 0)
            {
                SystemSounds.Beep.Play();
                MessageBox.Show("画像ファイルを選択して下さい。");
                return;
            }
            else if (ValidateOfRowColNumberTextBox() && CheckOfGraphicFile(filePathTextBox.Text))
            {
                SetParamters();
                Close();
            }
            else
            {
                SystemSounds.Beep.Play();
                MessageBox.Show("指定の画像ファイルは取り込みできませんでした。\n画像ファイルは65535ピクセルのjpg,png,bmp形式のみ取り込めます。");
            }
        }

        private void SetParamters()
        {
            if (null != FileName && int.TryParse(confRowTextBox.Text, out int row_num) && int.TryParse(confColumnTextBox.Text, out int col_num))
            {
                GraphicHeight = row_num;
                GraphicWidth = col_num;
                DialogResult = DialogResult.OK;
                return;
            }
            else
            {
                DialogResult = DialogResult.Cancel;
                return;
            }
        }
    }
}

そして、これを実際に呼び出します。
これはForm1にあたる場所に書かれていたものです。

namespace ClientForm
{
    public partial class MainForm : Form
    {
        public MainForm(string applicationName)
        {
            InitializeComponent();
        }

        private void OpenGraphChipButton_Click(object sender, EventArgs e) => ExecuteLoadGraphDialog(sender, e);
        private void GraphicChipPanel_DoubleClick(object sender, EventArgs e) => ExecuteLoadGraphDialog(sender, e);
        private void 開く_グラフィックチップ_Click(object sender, EventArgs e) => ExecuteLoadGraphDialog(sender, e);

        private void ExecuteLoadGraphDialog(object sender, EventArgs e)
        {
            using LoadGraphDialog loadGraphDialog = new();
            if (loadGraphDialog.ShowDialog() == DialogResult.OK && null != loadGraphDialog.FileName)
            {
                graphicChipPanel.BaseImage = Image.FromFile(loadGraphDialog.FileName);
                graphicChipPanel.LoadChipList(loadGraphDialog.GraphicHeight, loadGraphDialog.GraphicWidth);
                // ... 他のコントロールの更新処理など 省略
            }
            loadGraphDialog.Dispose();
        }
    }
}

（補足：Form1 → MainForm）

実際にはもっと色々コードは書いてありますが、説明のために省略してあります。
（元ソースコードも同じ）

全てメソッドには単一の機能を持たせてあり、クラスはおおよそ単一の責任のみを持たせています。
これは前回記事でも少し解説したSOLID原則「Simple Responsibility Principle：単一責任の原則」に則った結果です。

また、変数などのメンバは極力作成しないようにしてデータを減らしつつ、管理形態を維持します。
お気づきかと思いますが元コードにあった「Public Class ChipVolume」クラスは削除してあります。

それでいてそれらのデータはLoadGraphDialogからShowcasePanelへ渡しています。
これは「ExecuteLoadGraphDialog」メソッド内で全て行われます。
コード行は8行だけです。

以上がグラフィックデータパネル部分のリファクタリングになります。

継承を使ってあるべきものをあるべき場所へ、クラスの役割を明確にして可読性は向上したのではないでしょうか。

その他の機能

マップフィールドを読み取るパネル（左のパネル）についても、Panelオブジェクトを継承して独自のオブジェクトにしたものを配置しています。

後はUndoとRedo処理についても以前の記事で紹介させて頂きましたが、これもデザインパターンの「Commandパターン」で書き直しました。
これについては旧エディタで実装できてなかった機能です。

namespace ClientForm.src.Gems.Command
{
    public abstract class Command
    {
        public abstract void Execute();
        public abstract void Undo();
    }

    internal class MapTileChangeCommand : Command
    {
        public MapTileChangeCommand(TilingPanel member, ...)
        {
        }

        public override void Execute() => ChangeMapTiles(_memberA);

        public override void Undo() => ChangeMapTiles(_memberB);

        private void ChangeMapTiles(byte tileindex)
        {
            // ... 何かの処理
        }
    }

    internal class ChipSelectCommand : Command
    {
        public ChipSelectCommand(ChipManagedPanel member, ...)
        {
        }

        public override void Execute()
        {
            // ... 何かの処理
        }

        public override void Undo()
        {
            // ... 何かの処理
        }
}

これはオブジェクト指向プログラミングでは結構有名な多態性（ポリモルフィズム）を使っています。

この内容についてはまた後日詳細をお話したいと思いますので😊

リーダブルコードを書く上での注意事項

ここまでソースコードを弄くりまわしてきて、最後にちょっとだけお知らせというかコメントを書かせてください( `･∀･)ﾉ

基本的にオブジェクト指向プログラミングは、自由度の高い可能性を持ったパラダイム（枠組み）です。
プログラムを作るルールではない事に注意が必要です。

いわゆるプログラミング言語がプログラムの構造を決めるのではなく、プログラマーがプログラムの構造を決める権利を持っているという事です。

ちょっと分かりにくい表現かもしれませんが、例えば動物は自分の好きなものが目の前にあったらそれを食べると思います。
しかし我々は必ずしもそうしないと思います。
食べ物があってもそれを食べるかどうかはその人が決める事であり、食べ物が目の前にあるから食べるといった事はしないでしょう。それと同じ事です。

この思考に関して私が言いたいのは、VB.NETのコードを批判しまくってきましたが、それを使ってはいけないという意味ではないという事です。
（過去の私のコードは汚いのであれば非推奨ですが…(^^;ゞ）

選択肢としてあってもいいのでは？という気持ちで捉えてもらえればいいかなと思います。

正直なところ、私がそもそも2017年頃にVB.netでエディタを作ったのか、それを真剣に考えて当時も私は理由あってそうしたのだと思いますし、それまでC言語だけしか触った事がなかった身分でオブジェクト指向プログラミングを知りもしなかった私にとってVB.netはきっと魅力的に見えたのかもしれません。

リーダブルコードを書く事自体は実質現時点でベストな選択だと思います。
それは間違いありません。

しかし実際その技術を習得するには大変時間が掛かり、それに見合うだけの成果が見いだせるのか？という疑問もあります。
これについては仕方のない事です。

未だにExcel VBAマクロのコードで損益計算書を作ってる会社もあるでしょう。

そもそも論で今は便利な世の中ですから、データの管理をするだけなら開発能力がなくてもEXPLANNERとかSalesforceのような既存のパッケージを導入すればシステムは使えるのです。

ゲームのエディタだってUnreal Engineなんてものがありますよね。

プログラミングはそもそも楽しくないとやってられません。
それはなぜかと言うと、プログラミングはデザイン産業だからです。

はたまた私にデザインセンスがあるのか？と言われればないのですが、そんな私でも時間を掛ければなんとか形にできる事もある、そんな世界です。

話が逸脱してしまいましたが、読みやすいコードを書く事は、時間や予算、人員との兼ね合い、システムの規模によってその取捨選択が変わってきますよ、という事を言いたかったのです。

それは分かりやすいコードを書くも、一枚のファイルに全処理を書くもプログラマー次第で自由ですよ、という事です。

プログラミングを行う上でこんなやり方もあるよ、という趣旨でこの記事を書きました、という意思を分かってもらえればと思います( ′ω′)ﾉ

また間違った内容だよ、というのがあればコメント等で教えていただけるとありがたいです。
よろしくお願いいたします。
今日はこの辺りで🤗