コード可読性の向上: 分かりづらい変数名を変更したり、分かりづらい処理を同等のわかりやすい処理にするかコメントを書く

[kohei-noda-qcrg]

どういう状態か

@Yasuto-Masuda 増田さんと一緒に開発中、いつも同じ変数名(e.g. indmoとindmorなど)や処理でこれ何の変数(処理)だったっけ？となっていて、コードの可読性が低い状態です

やりたいこと

@minoria-hiroshima @Yasuto-Masuda
以上の問題点と #78 の解決のため、どこかで時間をとって議論させてください(リファクタリングを行っても構わないか、GitHubリポジトリの管理方法について話したいです)

[kohei-noda-qcrg]

具体的なコードの可読性の妨げになっている可能性のある記述、変数名を以下にメモしておきます

解決の基本方針

• 省略形が多く、それが可読性を下げる(意図が伝わらない)要因になるので、20文字程度の長さで、できるだけ省略しないように書く(書かれる回数が少ないので、サブルーチン名はもう少し長くても問題ないと思われる)
  • VSCodeや補完が効くほかのエディタで書いている限り、変数名をすべて書く必要はないので、書いているときは長めの変数名は大した問題にならない
  • 読むときは、あまりにも長い変数名だと読みづらくなることもあるので、折り合いをつける必要はある(長くなってしまう場合electronをelecと書く等)
• 適切にコメントをつける
  • よっぽど短いサブルーチンでない限りサブルーチンの目的などを書くとよい
  • 処理の中身に関しても直感的でない部分は説明か例かやっている内容の気持ちの部分をコメントとして書く
• 使わなくなった処理をコメントアウトで残さない
  • "適切にコメントをつける"のコメントとの違いがすぐにはわからなくなってしまうため
  • 個人的に残しておきたいなら、gitのstashで残しておくか、gitのコミットメッセージに消した処理の内容を書いておき、参照したくなった時にコミットメッセージで検索する

変数名

• indmo, indmor : MOのindexの変換であるが、DIRACからCASPT2への変換がどちらでCASPT2からDIRACへの変換がどちらかなのかが直感的にわかりづらい。mo_ind_dirac_to_caspt2, mo_ind_caspt2_to_dirac などの変数名が適切だと思われる。=>indmo_cas_to_dirac, indmo_dirac_to_casに変更
• 省略した変数名: ループのインデックスi,j,k,lなどは問題ないが、寿命が長め(サブルーチン全体で使うレベル)の変数名を省略形にすると、何を書いているのかがわかりづらくなってしまう(例: traint2はtransformed_2_electron_integralの略)

[kohei-noda-qcrg]

変数名

• あまりにも短すぎて分かりづらくなるものは変数名の変更を検討
• そうでないものは変数の定義部分に変数の説明を書いて、定義ジャンプで確認できるようにする
• 数式上インデックスが割り当てられているものはあまり変えないように

変更前と変更後をどうやって確認できるようにするか

• 変更したものの対応表をprとword版として2つ作っておく

[kohei-noda-qcrg]

以前の名前の書き方のフォーマット(候補)
caspt2_to_dirac ! [old name] indmo

[kohei-noda-qcrg]

idet => cas_idx, idetr => cas_idx_reverseに変更

[kohei-noda-qcrg]

プログラムのソースコードのファイル名

• r4dcasci_co.f90など、意味は分かるが冗長なものはcasci.f90などに改めたいと思っている。
• intra.f90など、省略が入っているものはtransform_integral.f90など一目見て意味が無理なく通るように変更したほうが良いと思っている。
• tyやcoなどのバージョン違いをファイル名で表すのはやめてバージョン違いを作りたければ、gitのbranchの機能を使ってわけて、mainにマージするときは、オプションやif文などで切り分けるなどする。

[kohei-noda-qcrg]

サブルーチン名

プログラムのソースコードのファイル名に同じ。
本プログラムでは、サブルーチン名は特に短すぎる場合が多い(Fortran77の時の制限?)
例: one_e_exctはone_electron_excitationにするなど。

※ rdiag,cdiagなど、realとcomplexという引数が違うだけのものはmoduleにしてmatrix_diagonalizationを呼べば、引数の違いによってrdiagとcdiagが切り替わるようにするとよい。(ただしuseが必要になる点は少し煩雑)

モジュール名

four_caspt2_moduleは実際にやっていることはglobalにしたい変数を定義している。何回も使っているので間違ったりはしないが、新しくコードを触る人には無駄に認知負荷をかけてしまう。global_variables_moduleなどとするのが良さそう

[kohei-noda-qcrg]

• Large scale code refactor #88 で解決済み