Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getter/setterの命名 #145

Closed
KowerKoint opened this issue Jul 23, 2024 · 3 comments · Fixed by #161
Closed

getter/setterの命名 #145

KowerKoint opened this issue Jul 23, 2024 · 3 comments · Fixed by #161
Assignees

Comments

@KowerKoint
Copy link
Contributor

PauliOperator::get_coefinternal::XGateImpl::targetなど、getterにはメンバの名前にget_がついているものとついていないものがある。
また、StateVector::amplitudesのようにgetterに見えて実際はタダのメンバ取得ではなく大きなコピーがかかっているものもある。
これらをわかりやすくなるために、以下の命名規則を採用する。

  • メンバに直接アクセスする場合、または実際にはmaskなど他の持ち方をしているがそこからの変換コストが低くメンバとして直接持っているかのように振る舞いたい場合
    • メンバと同じ名前の関数
    • 返り値はプリミティブ型やComplexのように十分コピーコストが小さければT、vectorのように大きければconst T&で返す
    • 可変にしたい場合、同名の非const関数でsetterを定義する。返り値はT&。もし本当はその形で持っていないならinternal::BitVector::_Referenceのように参照型のように振る舞うラッパークラスを返してもよいが、現状そういうものはなさそう。
  • 計算して結果を返す場合
    • get_つきまたはその他の適切な動詞型の名前をつける。
    • get_target_qubit_listget_target_qubit_maskに関してはどうするべきか要検討(これは統一して両方getでもいいかも?)
@gandalfr-KY gandalfr-KY self-assigned this Aug 27, 2024
@gandalfr-KY
Copy link
Contributor

get_target_qubit_list、get_target_qubit_maskに関してはどうするべきか要検討(これは統一して両方getでもいいかも?)

これはまさに「メンバに直接アクセスする場合、または実際にはmaskなど他の持ち方をしているがそこからの変換コストが低くメンバとして直接持っているかのように振る舞いたい場合」ではないですか?これに照らせばgetを付けない名前がよいのではないでしょうか。

@gandalfr-KY
Copy link
Contributor

#161

@KowerKoint
Copy link
Contributor Author

これに照らせばgetを付けない名前がよいのではないでしょうか。

ゲートによっては(Probablisticとか)取得が難しいというイメージだったけど、 だいたい一瞬だしProbablisticも今例外で今後コンストラクタでマスク作るくらいでいい気もするのでgetつけなくていいかも

@gandalfr-KY gandalfr-KY linked a pull request Sep 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants