マップエディタの開発後記です。
しばらくはこの関連の記事が続きます。
「VB.NETのコードを書き直す」という名のタイトルでシリーズ化してきてますが、このシリーズは途中で終了します。
なぜなら、元ネタであるVisual Basic.NETのソフトが未完成だからです😂
まさかの。
なので、途中から「マップエディタの製作」みたいな記事名に変わるんじゃないんですかね。
知らんけど( ^ω^)
書き直す理由
このシリーズでは再三かもしれないですが、一回話題を基点に巻き戻してみる事に。
そもそもなぜ書き直そうと思ったのか。
決定打になった理由はこれです↓
「なにこれ？」
って話なんですが、これは私が作ったVB.NETのゲーム制作用マップエディタです。
なんか真ん中に曰くのメッセージが見えますね。これはなんでしょうか？知らんがな。出るんやこれが
とまあ冗談はおいておき、こんな状態でゲーム制作などまともにできるわけもなく、私はこのアプリケーションの元ソース（プロジェクトファイル）を掘り起こして直そうと（一度は決心）しました。
しかしながら現実はあまりにも非情で、そこには殺伐とした光景が広がっていたのです。
これはひどいなんてものではありません。
またいつもの批評大会になってしまうのですが、
マジックナンバーだらけ、
一つのイベントハンドラに全てのロジックがつらつら書かれている、
変数が多すぎ、
そもそも変数名が理解できない暗号、
アクセス識別子が全部 Public でカプセル化なんて一切していない、
ただファイル分割してるだけ。
それはまるで構造化プログラミング以前の言語を眺めているかのような。そんなレベルでカオスすぎる光景でした。
一体誰がこんな糞コードを書いたのでしょうか？
私は身に覚えのない雑多なこのソースコードの編集を早々に諦めて、はじめから再構築する事にしました。
折角なので学習がてら、C#言語を使って作っていこうと企画を立てました。
そしてこのプロジェクトが始まります。
作り直すと言っても実際には元となるプログラムの主機能部分は同じで、マップエディタを作る事は変わりありません。
しかし元ソースがあの有様では参考にはならないので、ほぼ新規にプログラミングを開始します。
それと昔ウェブ上でとある記事を見た事があり、私はその記事を元にコーディングの基礎を再確認しながら進める事にしたのです。
それは「anopara」というぺージで、残念ながら今はもうブログが閉鎖されてしまっていて見る事はできませんが、その記事は「プログラミング中級者に読んでほしい良いコードを書く秘訣20箇条」（みたいなタイトルだった）というものでした。
ざっくり言うと、
- コードをなるべく書かない
- オブジェクト指向設計
を徹底すると良いコードになる、というものでした。
20箇条を実際に読んでいくと確かに納得できるものばかりで、私はその時作っていたコードがそれとは全く真逆の構造になっている事に落胆したのを今でも覚えています。
VB.NETのマップエディタはそれよりもずっと前に作ったもので、当然ながらコード量は多いし、オブジェクト指向プログラミングは一切しておりませんでした。
そもそも「オブジェクト指向」の意味が全く理解できなかった人間だったのです。
（今も分かってはないけどね🤪）
上記コードを見ても分かるように、とにかく思い通りに動けばそれでいいと言わんばかりな構造で、プログラムが動く感動だけに酔いしれていたのかもしれません(笑)
このプログラムが未完成なのは、上記の理解できないコードが入り組んでいる状態で開発者（当時の私）自身が制御できなくなったため、機能の追加（及び修正）が困難になったからではないでしょうか？
保守性の向上
以前までの記事でも紹介してきましたが、今回もこの手の話題を進めます。
プログラムは作ったら、作りっぱなしなんて事はまずあり得ないので、翌日以降にソースコードが読める状態でなければなりません。
どういう構造で作ったか思い出さないとプログラムコードのアルゴリズムが理解できない形だと保守性は低いと言えると思います。
例えばWindowsのエクスプローラーを起動すると以下のようなフォルダ類があると思います。
ダウンロードしてきたファイルは「ダウンロード」フォルダを、ネットプリントや作成したWord文書などは「ドキュメント」フォルダを、取得した音楽データは「ミュージック」フォルダを辿っていけばすぐお目当てのデータに辿りつきますよね。
それと同じ事で、いつ誰がどのように見ても目的地まで最短で辿り着けるような構造になっているのが理想形です。
例えば以下のようなPanelコントロールにたくさんのボタンを追加して並べる処理があります。
この処理は以下のコードにより実現されます。
public partial class ShowcasePanel : Panel
{
/// <summary>
/// Load chip list.
/// </summary>
/// <param name="rows">Number of lines in the chip list</param>
/// <param name="columns">Number of columns in the chip list</param>
public void LoadChipList(int rows, int columns)
{
const int GRAPHSIZE = CHIP_SIZE;
const int GRAPHBOXSIZE = GRAPHIC_BOX_SIZE;
const int CELLSIZE = CHIP_CELLSIZE;
if (null == BaseImage || 0 >= rows || 0 >= columns) return;
DeleteAllControl();
// Displays button controls with graphics on a panel at regular intervals.
for (int y = 0; y < rows; y++)
{
for (int x = 0; x < columns; x++)
{
Rectangle imgRect = new(x % columns / 1 * GRAPHSIZE, y * GRAPHSIZE, GRAPHSIZE - 1, GRAPHSIZE - 1);
Bitmap bitmap = new(GRAPHBOXSIZE, GRAPHBOXSIZE);
Graphics graphics = Graphics.FromImage(bitmap);
graphics.DrawImage(BaseImage, new Rectangle(0, 0, GRAPHBOXSIZE, GRAPHBOXSIZE), imgRect, GraphicsUnit.Pixel);
if (_chipManager!.AddImage((y * columns) + x, bitmap))
{
// Using a Factory Method pattern to get a button instance and downcasting to a custom button.
ButtonFactory buttonFactory = new ChipButtonFactory((y * columns) + x, bitmap);
IButtonProduct product = buttonFactory.GetProduct();
Button createButton = product.Create();
if (createButton is ChipButton button)
{
button.Location = new Point(x * CELLSIZE, y * CELLSIZE);
button.Click += Button_Click;
Controls.Add(button);
}
}
else
{
/*
* This syntax will pass if you try to register more than 255 chips, for example.
* This is because it falls under an exception pattern due to the structure of binary data.
*/
return;
}
}
}
}
}
他にも多くのメソッドやメンバを使っていますが、今そこは無視して下さい。
上記のVB.NET版元ソースコードは以下です。
Public Class GraphicChip1
''' <summary>
''' チップリストを生成します。
''' </summary>
''' <param name="imgFile">チップリストを作成する元イメージとなるファイルパス。</param>
''' <param name="col">作成するチップリストの列数を指定します。</param>
''' <param name="row">作成するチップリストの行数を指定します。</param>
''' <param name="fs">チップリストを表示するボタンの形式を指定します。</param>
''' <returns>チップリスト生成が正常に終了したかどうかを判別する Boolean 値。</returns>
Public Function Load_ChipList(ByVal imgFile As String, ByVal col As Integer, ByVal row As Integer, Optional ByVal fs As Boolean = False) As Boolean
Const ABS_LIST_SIZE As Integer = 48.0!
Const CHIP_LIST_MAX As Integer = 400
Const BUTTON_SIZE As Integer = 40
Dim panel As TableLayoutPanel = Form1.TableLayoutPanel1
Dim img As Image
' Process will check whether it is possible.
Try
img = Image.FromFile(imgFile)
Catch ex As IO.FileNotFoundException
MessageBox.Show("ファイルが見つかりません。処理を中断しました。", "通知")
Return False
Catch eex As Exception
MessageBox.Show("不明なエラーが発生したため処理を中断しました。", "通知")
Return False
End Try
If col * row > 400 Then
MessageBox.Show("チップ数が上限をオーバーしたため処理を継続出来なくなりました。チップ数の上限は「400」です。", "通知")
Return False
End If
' Create TableLayoutPanel1.
Dim columnvol = col
Dim rowvol = row
panel.ColumnCount = columnvol
panel.ColumnStyles.Clear()
For g As Integer = 0 To columnvol - 1
panel.ColumnStyles.Add(New ColumnStyle(SizeType.Absolute, ABS_LIST_SIZE))
Next
panel.RowCount = rowvol
panel.RowStyles.Clear()
For h As Integer = 0 To rowvol - 1
panel.RowStyles.Add(New RowStyle(SizeType.Absolute, ABS_LIST_SIZE))
Next
panel.Size = New Size(columnvol * ABS_LIST_SIZE, rowvol * ABS_LIST_SIZE)
Dim canvas(CHIP_LIST_MAX) As Bitmap
Dim graphicdata(CHIP_LIST_MAX) As Graphics
' この部分いらないはずです(要確認)
For h As Integer = 0 To CHIP_LIST_MAX
canvas(h) = New Bitmap(48, 48)
graphicdata(h) = Graphics.FromImage(canvas(h))
Next
' Divides the read image file and displays it sequentially in the individual space of TableLayoutPanel1.
Dim cpDivX As Integer = 0, cpDivY As Integer = 0, k As Integer = 0, propertyFlatStyle As FlatStyle, propertyFlatBorder As Integer
If fs = True Then
propertyFlatStyle = FlatStyle.Standard
propertyFlatBorder = 2
Else ' Default.
propertyFlatStyle = FlatStyle.Flat
propertyFlatBorder = 0
End If
Dim rest, src As Rectangle
For i As Integer = 0 To rowvol - 1
For j As Integer = 0 To columnvol - 1
Dim newButton = New Button
With newButton
.Name = "Chip" & k
.FlatStyle = FlatStyle.Flat
.FlatAppearance.BorderSize = 0
.Width = BUTTON_SIZE
.Height = BUTTON_SIZE
.BackColor = Color.Transparent
.BackgroundImageLayout = ImageLayout.None
.Tag = k
End With
cpDivX = ((j Mod columnvol) \ 1) * 16
cpDivY = i * 16
rest = New Rectangle(0, 0, 32, 32)
src = New Rectangle(cpDivX, cpDivY, 16, 16) 'Specify the size of the image chip.
graphicdata(k).DrawImage(img, rest, src, GraphicsUnit.Pixel)
newButton.BackgroundImage = canvas(k)
FAMESharedClasses1.Add_ChipData(newButton.BackgroundImage)
panel.Controls.Add(newButton, j, i)
panel.Invalidate()
AddHandler newButton.Click, AddressOf Func_newButton_Click
k += 1
Next
Next
Form1.TableLayoutPanel1 = panel
Form1.ChipEnterButton.Text = "リスト消去"
Return True
End Function
End ClassこのVB.NETのソースには多くの問題が含まれています。
まず最初に、Load_ChipList全体に複数の機能が含まれてしまっている事です。
これは最も万人から嫌われるプログラムのアンチテーゼです。
具体的には、元ファイルの読み込み、TableLayoutPanelコントロールの作成、ボタンオブジェクト（コントロール）の作成、画像の描画、イベントハンドラの追加です。
メソッドは一つにつき一つの機能のみを有するべきであり、それが故にメンテナンスを容易にします。
そしてこのコードの致命的なミスはこのメソッド内で作ったTableLayoutPanelをメインフォームに追加している事です。
それはメインフォームの中に配置されるPanelコントロールの主従関係が崩壊している証拠であり、Load_ChipListメソッドがMainFormのコントロールの仕様を定義してしまっています。
TableLayoutPanel1はMainFormのコントロールで、グラフィックのチップリストを生成するビヘイビアー（挙動）自体には何も相容れないはずです。
それは例えば、グラフィックのチップリストを作る対象がTableLayoutPanelではなく、Panelに変わったらどうなるでしょうか？
それはもうLoad_ChipListメソッドが大規模なリフォーム工事⛏を行う現実に直面する事になります。
後はクラスやメンバの名前ですね。
「GraphicChip1」って流石に適当に付けたとしか…(´・ω・`)
そもそも”1″とか番号を付けるべきではない事は明らかで、これは恐らくVisual StudioなどのIDEが自動的に制定するファイル名をそのまま流用しているのではないかと思われます。
マジックナンバーも大量に書かれているので、もうこれは既に作った本人以外にはまず読めないでしょう。
オブジェクト指向設計について考える
これはオブジェクト指向プログラミングの基礎的な情報としてとても重要な概念です。
ここで少しその概念をおさらいします。
- Single Responsibility Principle : 単一責任の原則
- SOLID原則の一つ。クラスなどのモジュールは一つの責任だけを担うようにしましょう、という意味です。
- Open-Closed Principle : オープン・クローズドの原則
- SOLID原則の一つで、これはコードの改訂を行う際に既存のコードを直接変更するのではなく、コードに追記するようにしましょうというものです。
- Liskov Substitution Principle : リスコフの置換原則
- これは少し概念が曖昧で難しいですが、若干緩い解釈を含めて言うと親のインターフェースに対して子どもであるクラスが無尽蔵に機能を覚えてどんどん変貌していき、本来の役割から逸脱した存在になってしまう現象を抑制するための規定の事です。一般的にインターフェースなどの抽象クラスは具象クラスとして利用する実装を定める役割を持っている必要があるため、このルールを守りましょう、という事です。is-a関係。
- Interface Segregation Principle : インターフェース分離の原則
- これもSOLID原則の一つです。さきほどのリスコフの置換原則と若干説明が関連しますが、インターフェースなどの抽象クラスが誇大化して、他の具象クラスから利用したくても不要な機能の実装を強要されてしまう問題を回避するためのものです。単一責任の原則にも通じる概念かと思います。
- Dependency Inversion Principle : 依存関係逆転の原則
- SOLID原則の中でも恐らく一番重要な（ものだと私は思っている）原則です。これはインターフェースとクラスは親子関係であくまでも子は親に依存すべきで、親が子に依存してはいけないという概念です。さきほどのTableLayoutPanelの話がこの概念の違反パターンです。インターフェースがある具象クラスに依存してしまっている場合、その具象クラスの仕様が変わるとインターフェースも修正しないといけなくなり、更にはそのインターフェースを継承・実装している他の具象クラス・・・それはもう考えたくない事態ですよね😅
- カプセル化
- これはプログラミングをかじった事ある方なら恐らく知っていると思われるワードですが、いわゆるオブジェクトが保有する資産（メンバなど）を隠蔽・秘匿して、外部には必要最低限の機能だけを公開しましょうとする手法の事です。私もこの概念は常に考えるようにしていて、この概念の真逆にはグローバル変数問題があると思います。
- KISSの原則（Keep it simple stupid.）
- stupidと書いてますが、別に悪い事じゃなくシンプル・イズ・ベストという言葉にあるように、簡潔な実装がメンテナンス性を向上させるよ、という事を体現した原則です。つまり一つのメソッドに複数の機能が含まれているのは推奨されません。
- デザインパターン
- デザインパターンはプログラミングを行う上で非常に有利になるコーディング技術で、GoFのデザインパターンなどとも呼ばれます。有名なのは私もよく使うFactory MethodパターンやStateパターン、Flyweightパターンなどです。
他にもいくつかオブジェクト指向プログラミングの定石があるようです。
それらはオブジェクト指向プログラミングを行う上でかなり有用な開発支援技術として利用できると思います。
ドメイン駆動設計をする
では色々ある中でどうやって作るのが解なのかという話ですが、オブジェクト指向プログラミングは幅が広く、作ろうとしているシステムやアプリケーションによって解法は異なります。
だから、巷でOOPは難しい、難解だ、などと揶揄される事も多いのでしょう。
それではどうすべきかというと、やはり要求事項に限りなく近い設計手法を取る事です。
それはつまり、ユーザーの要望に沿った形の設計をする事で、仕様変更や不具合の対応に柔軟に対応できる体制を作り上げる事を意味します。
そりゃそのほうがいいですよね。人間だもの。みつを
結局のところ、設計ってなんのためにするかと言うとそれは手戻りを少なくするため、
言い換えると直接結果に結びつかない浪費を低減させるためにするものだからです。
最初からシステム開発者がエンドユーザーの要望を聞いて、その場で設計して開発をすればいいんですよ。
バックエンドと言いながらも、フロントサイドに立って要件定義と開発を結び付ける。
それこそが正に本来あるべきシステム開発のあるべき姿だと私は考えます。
近年、日本の多くの企業でIT化が進む中、自社開発を選択する企業も増えていると聞きます。
それはこのシステム開発効率化を考えての事ではないかと思います。
まあ、他のシステム会社に委託したら精度いい物は返ってくるけど、コスト高いもんね( ^ω^)
ずっとシステムの面倒みてもらうわけにもいかないし。
最近流行りのノーコード開発ツールはこういった社会全体の流れが必然的に生み出した産物なのかもしれません。
今のご時世、ローリスク・ローリターンの慎重な動きに社会全体がなってきてる感じがします。
やはり色々な事件や事故などが露呈する事が増えて、危機意識も高くなってきている証拠なんでしょうかね。
ん？なんか話変わってきてるから、今日はこの辺で閉じます。
コメント