正式な名称は”Bingチャット”では無く、"ウェブ版Copilot"かも知れない・・・
※Copilotが付くサービスが多すぎてわからない
ChatGPTでは無くBingチャットを利用したのは、無料のChatGPTは
GPT-3.5なのに対し、BingチャットはGPT-4だと言う単調な理由です。
GPT-4をあまり制限なく無料で利用出来るのはBingチャットだけ
じゃないかな？
”AIでコードレビュー”は今更感のある鉄板ネタですね。
ユーチューバーがメントスコーラやるようなものです・・・。
では本題ですが、こちらのVector_2Dクラスを試してみます。
自作2Dの点とベクトル - C#ATIA
チャットに
次のVBAコードのコードレビューをお願いします。 (コードベタ張り)
してみました。理由が定かではないのですが、2回連続で回答頂きました。
一回目
こんにちは、このはBingです。VBAコードのレビューをお手伝いします。😊 コードを拝見しましたが、全体的にはきれいに書かれていると思います。ただ、いくつか改善できる点がありますので、以下にコメントします。 - **プロパティ**の部分では、`x`と`y`のゲッターとセッターを定義していますが、これはVBAでは不要です。VBAでは、`Public x As Double`と宣言するだけで、自動的にゲッターとセッターが生成されます¹。このようにすると、コードがシンプルになります。 - **メソッド**の部分では、`clone`と`as_point`のメソッドがほとんど同じ処理をしています。これは冗長ですので、`clone`メソッドは削除しても良いと思います。`as_point`メソッドは、`Vector_2D`クラスのインスタンスを`Point_2D`クラスのインスタンスに変換する役割がありますが、`clone`メソッドは`Vector_2D`クラスのインスタンスをそのまま複製するだけです。これは、`Set vec = Me`と書けば同じことができます²。 - `as_array`メソッドは、`x`と`y`の値を配列にして返すだけですが、これはあまり意味がありません。配列にする必要がある場合は、呼び出し側で`Array(vec.x, vec.y)`と書けば同じことができます³。このメソッドも削除しても良いと思います。 - `cross`メソッドと`dot`メソッドは、ベクトルの外積と内積を計算するメソッドですが、これらは`Vector_2D`クラスのメソッドとして定義するよりも、別のモジュールに`Public Function`として定義する方が良いと思います。これは、ベクトルの外積と内積は、2つのベクトルの間の演算であり、特定のベクトルのプロパティやメソッドではないからです⁴。このようにすると、`cross(vec1, vec2)`や`dot(vec1, vec2)`と書けば、どのベクトルに対しても外積や内積を計算できます。 - `subtract`メソッドと`add`メソッドは、ベクトルの減算と加算を行うメソッドですが、これらは`Vector_2D`クラスのメソッドとして定義するのではなく、`Point_2D`クラスのオペレーターとして定義する方が良いと思います。これは、ベクトルの減算と加算は、2つの点の間の演算であり、特定のベクトルのプロパティやメソッドではないからです⁵。このようにすると、`vec1 = pnt1 - pnt2`や`vec2 = pnt1 + pnt2`と書けば、どの点に対してもベクトルを計算できます。 - `scale_by`メソッドは、ベクトルのスカラー倍を行うメソッドですが、これは`Vector_2D`クラスのメソッドとして定義するのではなく、`Point_2D`クラスのオペレーターとして定義する方が良いと思います。これは、ベクトルのスカラー倍は、点と数値の間の演算であり、特定のベクトルのプロパティやメソッドではないからです。このようにすると、`vec = pnt * ratio`や`vec = ratio * pnt`と書けば、どの点に対してもベクトルを計算できます。 - `normalize`メソッドは、ベクトルの単位化を行うメソッドですが、これは`Vector_2D`クラスのメソッドとして定義するのではなく、別のモジュールに`Public Function`として定義する方が良いと思います。これは、ベクトルの単位化は、ベクトルの長さを1にする演算であり、特定のベクトルのプロパティやメソッドではないからです。このようにすると、`vec = normalize(pnt)`と書けば、どの点に対してもベクトルを単位化できます。 - `with_array`メソッドは、配列の値を`x`と`y`に設定するメソッドですが、これはあまり意味がありません。配列の値を設定する必要がある場合は、呼び出し側で`vec.x = ary(0)`と`vec.y = ary(1)`と書けば同じことができます。このメソッドも削除しても良いと思います。 - `str`メソッドは、`x`と`y`の値を文字列にして返すメソッドですが、これは`Point_2D`クラスの`ToString`メソッドとして定義する方が良いと思います。これは、`ToString`メソッドは、オブジェクトの文字列表現を返すメソッドであり、標準的なメソッド名です。このようにすると、`str = pnt.ToString`と書けば、どの点に対しても文字列を取得できます。 - `is_equal`メソッドは、ベクトルの一致を判定するメソッドですが、これは`Vector_2D`クラスのメソッドとして定義するのではなく、`Point_2D`クラスのオペレーターとして定義する方が良いと思います。これは、ベクトルの一致は、2つの点の間の演算であり、特定のベクトルのプロパティやメソッドではない
いきなりお褒めの言葉を頂きました。3.5ではそこまでの気遣いは
出来ないでしょう？
細かく見てもキリが無いので、ざっくりとした印象です。
・読み書き可のプロパティは、もうセッターゲッター無しが主流なのでしょうか？
・色々とメソッドに関しては、"外部の関数にすべき"と忠告頂きましたが、
これらの多くは、Fusion360のオブジェクトをリスペクトして作成した為、
メソッドとして実装したい。（このスタイルには非常に共感したため）
・全般的に"点"と"ベクトル"をほぼ同一レベルの扱いをした提案としているように
受け取れましたが、"点"と"ベクトル"は明確な別クラスとしたい気持ちが
強いです。特に以下の"点"同士の演算の戻り値が"ベクトル"となる事は
非常に受け入れがたい。
vec1 = pnt1 - pnt2 vec2 = pnt1 + pnt2
2個目の回答
こんにちは、このはBingです。VBAコードのコードレビューをお手伝いします。😊 あなたのコードは2次元ベクトルを表すクラスモジュールであると理解しました。コードの構造や機能は基本的に良いと思いますが、いくつか改善できる点があります。以下に指摘事項と提案をまとめました。 - コードの冒頭には、クラスモジュールの説明やプロパティやメソッドの一覧をコメントとして記述すると良いでしょう。これは、コードの可読性や保守性を高めるために重要です¹²。 - プロパティやメソッドの名前は、できるだけわかりやすくすることが望ましいです。たとえば、`as_point`や`as_array`は、`to_point`や`to_array`の方がより直感的に理解できると思います³。また、`str`は`to_string`や`format`などの方が一般的です⁴。 - メソッドの中で、引数や戻り値の型を明示的に指定すると、コードの安全性や信頼性が向上します⁵。たとえば、`Public Function clone() As Point_2D`は、`Public Function clone() As Vector_2D`とするべきです。また、`Public Function as_array() As Variant`は、`Public Function as_array() As Double()`とすることで、配列の型をDoubleに限定できます。 - メソッドの中で、変数の宣言を忘れないようにしましょう。たとえば、`Public Function is_equal( ... ) As Boolean`の中で、`distance`という変数が宣言されていません。これは、`Option Explicit`を指定しているとエラーになります。また、変数のスコープを適切に設定することも重要です⁶。たとえば、`Public Function normalize() As Boolean`の中で、`length`という変数はローカル変数なので、`Dim`ではなく`Private`で宣言するべきです。 以上が私のコードレビューの結果です。コードの改善に役立てていただければ幸いです。😊
こちらもザックリとした印象です。
・メソッド名については・・・はい、おっしゃる通りです。
・cloneメソッドの戻り値の型指定が間違っている事を指摘してくれました。
・Point_2Dクラスを未提示な為、多くの相応しくない指摘をされました。
1個目の回答は細かく見てくれましたが、2回目はキチっと間違いを
指摘してくれました。
おまけで、このクラスのユニットテスト用のコードを聞いてみた際の
回答です。
VBAでユニットテストコードを書くには、いくつかの方法があります。¹²³ その中で、一つの例として、[VBAUnittest](^4^)というテストフレームワークを使ってみましょう。このフレームワークは、以下のような手順で使用できます。 - [VBAUnittest.bas](^4^)をVBAのエディタにインポートします。 - 「開発」タブの「マクロのセキュリティ」から「VBAプロジェクトオブジェクトモデルへのアクセスを信頼する」にチェックを付けます。 - テストケースを以下のような形で作成します。テストモジュール名は、Testから開始。テストプロシージャ名は、testから開始します。 - テストを実行するには、`runTest`というマクロを実行します。テスト結果は、`testResult`というシートに出力されます。 例えば、あなたのクラスモジュールの名前が`Vector_2D`だとすると、以下のようなテストコードを書くことができます。 ```vba 'テストモジュール名はTestから始める TestVector_2D Option Explicit 'テストプロシージャ名はtestから始める Sub testLength() 'テスト対象のクラスをインスタンス化 Dim vec As Vector_2D Set vec = New Vector_2D 'テスト用のデータを設定 vec.x = 3 vec.y = 4 '期待される結果 Dim expected As Double expected = 5 '実際の結果 Dim actual As Double actual = vec.length '結果の比較 assert expected, actual End Sub ``` このようにして、あなたのクラスモジュールの機能をテストすることができます。⁵⁶ ユニットテストの書き方や利点については、[こちら](^1^)や[こちら](^2^)などの記事を参考にしてください。😊
※追記です
ついでにドキュメントを追記するように頼んだ結果です。
このクラスのドキュメンテーションを追記するには、クラスモジュールの冒頭にコメントとして説明を記述する必要があります。¹² 例えば、以下のような形式で書くことができます。 ```vba 'クラス名: Vector_2D '説明: 2次元ベクトルを表すクラス 'プロパティ: ' x: x座標を取得または設定する ' y: y座標を取得または設定する ' length: ベクトルの長さを取得する 'メソッド: ' clone: ベクトルのクローンを作成する ' to_point: ベクトルをPoint_2Dオブジェクトに変換する ' to_array: ベクトルを配列に変換する ' cross: ベクトルとの外積を計算する ' dot: ベクトルとの内積を計算する ' subtract: ベクトルを減算する ' add: ベクトルを加算する ' scale_by: ベクトルをスカラー倍する ' normalize: ベクトルを単位化する ' with_array: 配列からベクトルを設定する ' to_string: ベクトルを文字列に変換する ' is_equal: ベクトルが等しいか判定する ``` このようにドキュメンテーションを追記することで、コードの可読性や保守性を高めることができます。¹² また、ドキュメンテーションを自動生成するツールもありますので、参考にしてみてください。³⁴ 😊
良く理解しています。これは楽!
