読者です 読者をやめる 読者になる 読者になる

C#ATIA

↑タイトル詐欺 主にCATIA V5 の VBA

指定ボディを子ボディにしつつ、リネーム2

CATIA_V5 VBA KCL

こちらの続き・・・と言うか反省です。
指定ボディを子ボディにしつつ、リネーム - C#ATIA


昨日のコードでは、こう書きました。

'vba type1
Sub CATMain()
    Dim Msg$: Msg = "Bodyを選択して下さい : ESCキー 終了"
    Dim SelElem As SelectedElement
    Do
        Set SelElem = KCL.SelectElement(Msg, Array("Body"))
        If KCL.IsNothing(SelElem) Then Exit Do
        Dim BodyOj As Body: Set BodyOj = SelElem.Value
        Dim Pt As Part: Set Pt = KCL.GetParent_Of_T(BodyOj, "Part")
        
        'パーツボディチェック
        If IsMainBody(BodyOj, Pt) Then
            MsgBox "パーツボディは出来ません"
            GoTo Continue
        End If
        
        '子ボディチェック
        If BodyOj.InBooleanOperation = 1 Then 'Boolean型だけどTrueじゃNG
            MsgBox "既に子ボディの為出来ません"
            GoTo Continue
        End If
        
        Call InitNewBody(BodyOj, Pt)
        Pt.Update
Continue:
    Loop
End Sub

GoTo文を使っているのが悪いとかじゃないです。ループをスキップする
Continue文がVBA無い為に使っただけなので。

処理を日本語で書くと

・パーツボディの場合は処理しない
・子ボディの場合は処理しない
・そうでなきゃ、処理します

と言う感じです。


ブログを始める前、3年ぐらい前ならこんな書き方を平気でしていました。

'vba type2
Sub CATMain()
    Dim Msg$: Msg = "Bodyを選択して下さい : ESCキー 終了"
    Dim SelElem As SelectedElement
    Do
        Set SelElem = KCL.SelectElement(Msg, Array("Body"))
        If KCL.IsNothing(SelElem) Then Exit Do
        Dim BodyOj As Body: Set BodyOj = SelElem.Value
        Dim Pt As Part: Set Pt = KCL.GetParent_Of_T(BodyOj, "Part")
        
        'パーツボディチェック
        If IsMainBody(BodyOj, Pt) Then
            MsgBox "パーツボディは出来ません"
        Else
            '子ボディチェック
            If BodyOj.InBooleanOperation = 1 Then 'Boolean型だけどTrueじゃNG
                MsgBox "既に子ボディの為出来ません"
            Else
                '処理
                Call InitNewBody(BodyOj, Pt)
                Pt.Update
            End If
        End If
    Loop
End Sub

悪名高いGoTo文は使用していませんが、if文の中にif文が入っており
インデントが深いです。この処理を日本語で書くと

・パーツボディじゃなくて、子ボディじゃない場合は処理します

と言う感じです。


ど素人でVBAユーザーな僕が、コーディングについて語る資格も能力も無いのですが
恐らく同様な結果が得られるのであれば、コードに正解は無いような
気がしています。 但し「ベスト」は判らないのですが、「ベター」なものは
ある程度存在しているような気はしています。

「綺麗なコード」の定義があいまいなのですが、「この様にした方が良いよ」と
されるものの一つに「インデント深くしない」と書かれている事を良く見かけます。
(もちろんインデントすべきところは、行った方が見やすいです)
検索してもらえば、「関数化する」とかイロイロとヒントとなる事を見つけることが
出来ると思うのですが、個人的に非常に強く印象に残ったのは
「処理しないものを、最初に除去する」
と言う事でした。(何処で読んだか、忘れました)

type2の場合は、if文を使って処理すべきものを else内の送り込んで
最後に処理させています。 当時であれば、僕の脳みそではそのように考えて
いましたし、それ以外は思いつきませんでした。
この方法では「処理しないものを、最初に除去する」にはなっておらず
「処理するものを探し出す」の状態になっており、これが嫌だったため
type1のコードを昨日書きました。 事実type1の方が、インデントは浅いです。


で、チョロチョロ書きましたが今朝思い付いた、今の僕の知識で書けるベターなコードは
こんな感じです。

'vba type3
Sub CATMain()
    Dim Msg$: Msg = "Bodyを選択して下さい : ESCキー 終了"
    Dim SelElem As SelectedElement
    Do
        Set SelElem = KCL.SelectElement(Msg, Array("Body"))
        If KCL.IsNothing(SelElem) Then Exit Do
        Dim BodyOj As Body: Set BodyOj = SelElem.Value
        Dim Pt As Part: Set Pt = KCL.GetParent_Of_T(BodyOj, "Part")
        
        Select Case True
            'パーツボディチェック
            Case IsMainBody(BodyOj, Pt)
                MsgBox "パーツボディは出来ません"
            '子ボディチェック
            Case BodyOj.InBooleanOperation = 1 'Boolean型だけどTrueじゃNG
                MsgBox "既に子ボディの為出来ません"
            '処理
            Case Else
                Call InitNewBody(BodyOj, Pt)
                Pt.Update
        End Select
    Loop
End Sub

GoTo文も擬似Continue文(Continueも結構悪名高い)も消せました。

よく考えると「Select Case True」は結構微妙で、与えられる条件
の「Case ~」の中身は、統一感が無くても書けてしまいます。
C#で同様のコードを書こうとした時、出来なかったのですが

C#でお手軽に言語判定を行いたい - C#ATIA

今更ながら、その辺がC#としては許さなかったのかなぁとも感じます。
VBAではエラーにはならないので、言語仕様的にOKなのだからギリセーフかな。