s2e-core
s2e-core copied to clipboard
Consider namespace separation rule
Overview
Consider namespace separation rule
Details
Discussed at
https://github.com/ut-issl/s2e-core/pull/321#discussion_r1104400947
Conditions for close
NA
Supplement
NA
Note
NA
ライブラリディレクトリが整理されたので新しい構成に合わせて名前空間も見直す。 ディレクトリ構成に合わせるなら、次のようになるか?
とりあえず、今存在しているlibra
に焦点を当てて考える。library
ディレクトリがほぼそのままmath_physics
に変わったので、次のような感じか?
namespace math_physics{
namespace math{
Class Vector;
Class Matrix;
他も同様
}
namespace atmosphere{
Class HarrisPriester;
Class Nrlmsise00;
}
}
もしくは、
namespace math_physics{
Class Vector;
Class Matrix;
Class HarrisPriester;
Class Nrlmsise00;
}
や
namespace math{
Class Vector;
Class Matrix;
他も同様
}
namespace atmosphere{
Class HarrisPriester;
Class Nrlmsise00;
}
などもあり得るのか?コメントいただきたい。
math_physics
は名前が長い割に責務が明確ではない(math
と physics
それぞれの責務のモノがなんだかんだ独立して存在する)ので、
namespace math{
Class Vector;
Class Matrix;
他も同様
}
namespace atmosphere{
Class HarrisPriester;
Class Nrlmsise00;
}
のような分割はかなり自然に思います。
コメントありがとうございます。私も下記のものは良いと思っているので、この方向で進めようと思います。
namespace math{
Class Vector;
Class Matrix;
他も同様
}
namespace atmosphere{
Class HarrisPriester;
Class Nrlmsise00;
}
#658 をreviewしてて感じたことです。
Orbit
class があるのに,それは orbit
namespace 外なことに少し違和感がありました。
library directory がなくなって math_physics が他の src 下の directory と同列になったこともあり,他全体についても namespace があるとよい気がしました。例えば component::Component
disturbance::GravityGradient
といった感じです。
この際に,math_physics/orbit と dynamics/orbit とを区別する必要が出てきます。
自然なのは directory に即した区別で,例えば math_physics::orbit::OrbitalElements
dynamics::orbit::RelativeOrbit
のような感じになります。
ただ,これは上記で棄却された案で,冗長でもあるので,悩ましいですね。
@conjikidow コメントありがとうございます。とりあえず今出ているPR消化してから改めて整理して考えたいのでレビューお願いします。
@conjikidow こちらの議論ですが、今は次のような状況です。
- 元々
namespace
は旧libraryにしかついていなかったので、今の整理も旧library関係のもの(主にmath_physics)になっている
旧library以外のもの全てにnamespaceをつけるかの議論がまず必要だと思います。 全てにnamespaceをつける利点・欠点などは整理したいですが教えてもらえると嬉しいです。
参考として、dynamics/orbit
とmath_physics/orbit
の差としては、次のようなことを考えています。
- 前者は真値系としての実体であり後者を利用して実体化させたものである。
- 後者はあくまでいろんな人に使われる側であり、s2e-coreでは前者に使われている。ユーザーによってはこれを利用して推定系や制御系など真値系と別の
estimate/orbit
などを作る可能性もある
@200km ありがとうございます。
元々namespaceは旧libraryにしかついていなかったので、今の整理も旧library関係のもの(主にmath_physics)になっている
この経緯は理解しているため,今進められているもの自体についてはそのままで構いませんが,今後のさらなるアップデートに向けて,という意図でした。 その上で
旧library以外のもの全てにnamespaceをつける
のほうが良いと考えています。
全てにnamespaceをつける利点・欠点などは整理したいですが教えてもらえると嬉しいです。
ですが,
- 全てにnamespaceをつける利点
- 衝突を避けられる
- 現在,旧library以外のものはglobalになっているため,名前衝突のリスクがある
- 例えば
dynamics/orbit
math_physics/orbit
estimate/orbit
の中で同じ名前のものを作りたい可能性もあるが,現状はそれができず名前を変えることが必須である
- 例えば
- ユーザーが独自の外部ライブラリを利用する場合にも衝突の可能性がある
- 現在,旧library以外のものはglobalになっているため,名前衝突のリスクがある
- ユーザーの自由度が増す
- 上に関連するが,現状はglobalになっているために,ユーザーがcore側で提供されているクラスや関数と同名のものを定義することができない
- 正確にはユーザーが独自の名前空間を用いれば定義可能だが,その場合もcore側はglobalなので
::ClassName
のようにglobal名前空間を明示的に示す必要があり,ambiguousである
- 正確にはユーザーが独自の名前空間を用いれば定義可能だが,その場合もcore側はglobalなので
- 特にその名前が一般的であるほど名前に自由度がない
- 例えば独自の電池を扱いたい場合,すでにcoreで
Battery
が定義されているため,それを継承したクラスを作ろうとしてもBattery
は使えない - 命名規則に反する
Bat
BAT
のようにするか,CustomBattery
などのようにするか,あるいは製品名にするか,など,他にも可能な選択肢はあるが,ユーザーにそれを強要することになる
- 例えば独自の電池を扱いたい場合,すでにcoreで
- 上に関連するが,現状はglobalになっているために,ユーザーがcore側で提供されているクラスや関数と同名のものを定義することができない
- 構造が明確になり可読性が上がる
- 例えば書いていただいたような違いが
dynamics/orbit
とmath_physics/orbit
があるが,現状それらの区別はクラス名からしかわからない- math_physicsは旧libraryに該当するので実際にはそうではないが,ユーザーが estimate/orbit を定義する場合にはそうなる
- 例えば
math_physics::orbit
dynamics::orbit
estimate::orbit
のようにすれば,それぞれの所属が明確になる(もちろんestimate::orbit
についてはユーザーの自由だが)
- 特にこの構造をファイル構造と一致させた場合,関連するコードがどこにあるかが一目でわかるようになる
- 例えば書いていただいたような違いが
- 衝突を避けられる
- 全てにnamespaceをつける欠点
- 修正コストがかかる
- core はもちろん,現在のユーザーも作業が必要になる
- 後方互換性がなくなる
- ユーザーがその作業をしない限りcoreの後方互換性が効かない
- 冗長になる
- 単純に名前が長くなる
- 修正コストがかかる
といったところでしょうか。
小規模なプロジェクトでは必ずしもそうではないですが,大規模で長期的なプロジェクトにおいては維持や拡張性の観点から名前空間を導入するほうが一般的だと思っています。 一方で欠点に挙げたコストがかなり大きく無視できないので,そことのトレードオフかなと思います。
具体的にありがとうございます。基本的な利点・欠点の整理について、書かれている内容は同意します。あとは、程度問題で何に重きを置くかですね。
現状のaobcなどを考えて個別の事象にどう対処しているかを考えながら議論を深めたいですね。
例えば独自の電池を扱いたい場合,すでにcoreで Battery が定義されているため,それを継承したクラスを作ろうとしても Battery は使えない
などは、製品名にする
ことを推奨しているという気持ちです。(aobcなどは全てそう)
また、これをs2e-core側でcomponent::battery
としても、一般性が高いままなので、ユーザーも結局component::battery
使いたいとなる気もしています。
ユーザー側が、my_component::battery
などにするなら、my_battery
とするのとあまり変わらないですから難しいですね。s2e-core側をcore_component::battery
のように一般性を下げる方が良いんですかね。
dynamics::orbit
も、ユーザーからしたら推定器の中のdynamicsだから、dynamics::orbitと名付けたいな
というのなども十分あり得る気がしています。
この辺り、s2e-core側で良い名前空間をつけないと、欠点にあるような労力をかけたのに、結局利点にあるようなスマートな名前衝突回避にならず、名前空間使わずに衝突回避した時と変わらない
とかになってしまいそうなのを危惧しています。
これを根本解決するには、全てを s2e-core::hoge::fuga
のように絶対ユーザーがつけないようにするとかしか無いんですかね。
ちなみに、欠点の後方互換性と名前が長くなる件は、あまり重要視していません。
- どのみちこのMajor updateはいろんな後方互換性をなくしている
- 名前が長くなる件はs2e-coreでは許容して、略語なども使わないという方針になっている
ありがとうございます。
これをs2e-core側でcomponent::batteryとしても、一般性が高いままなので、ユーザーも結局component::battery使いたいとなる気もしています。 ユーザー側が、my_component::batteryなどにするなら、my_batteryとするのとあまり変わらないですから難しいですね。s2e-core側をcore_component::batteryのように一般性を下げる方が良いんですかね。
この辺り、s2e-core側で良い名前空間をつけないと、欠点にあるような労力をかけたのに、結局利点にあるようなスマートな名前衝突回避にならず、名前空間使わずに衝突回避した時と変わらないとかになってしまいそうなのを危惧しています。
これを根本解決するには、全てを s2e-core::hoge::fugaのように絶対ユーザーがつけないようにするとかしか無いんですかね。
これはおっしゃる通りで,究極は
全てを s2e-core::hoge::fugaのように
core全体を特定の名前空間下に置く必要があるという結論になると思います。
これ自体は不自然ではなく,std
boost
Eigen
等々,主要なライブラリでも一般的な方法です。
ただ,この議論と,個々のクラスの名前空間をどこまで深くするかは別の議論になる気がしますね(現状において全て s2e-core
下に置くだけという方針もあり得るため)。
いずれにせよ,少なくともv8は外部ライブラリ周りの修正がメインのため,今回修正いただいたところで閉じて良いと思います。
あとは、程度問題で何に重きを置くかですね。
の通りで絶対正しい答えがあるわけではないですし,すぐに結論が出るものでもないと思うので。 自分が道を外したコメントをしてしまって恐縮ですが,当初の旧ライブラリ関連については結論が出ているので,こちらは完了で問題ないかと思います。
修正方針が決まれば修正自体は簡単なので、今回やってしまっても良いかなと思っています。
s2e::ディレクトリ名
のようなルールで行くとすぐ決めて良いならそれでやってしまいます。いかがでしょうか?
ありがとうございます。
欠点の後方互換性と名前が長くなる件は、あまり重要視していません
ということであれば,自分も s2e::ディレクトリ名
で良いかなと思います。
では、それでPR出してみようと思います。
ありがとうございます,よろしくお願いします。