下記に従うようコーディングを行ってください.
初めに一読しておいてください.
新規にコードを書くときは ROS標準のコーディング規約に従うようにしてください.
https://github.com/davetcoleman/roscpp_code_format
上記のサイトに ROS C++コーディング規約のための, .clang-formatファイルがある. このファイルを利用することで, インデントサイズ, カッコ位置等のスタイルに 関する規約については自動で準拠することができる.
- clang-formatをインストールする(Ubuntuだとパッケージあり). 新しいバージョンが良い
- プロジェクトのトップディレクトリに .clang-formatファイルを置く
clang-format -i filenameとすると .clang-formatに従いフォーマットしてくれる
clang-format -iだとファイルは上書きされるので, 実行前に "git commit"を
しておくと安全である.
システム全体に適用するには以下のようなシェルスクリプトを実行すればよい.
for file in $(git ls-files | \grep -E '\.(c|cpp|h|hpp)$' | \grep -v -- '#')
do
clang-format -i $file
donePEP8準拠をチェックするツールは pep8などで機械的に行えばよい. 現状の ROSは Python2.5をターゲットとしているようだが, Ubuntu 16.04以降では デフォルトの Pythonのバージョンが 3になるので, 今後の保守を考えると, Python3準拠で書いておいた方が良いと思う.
- パッケージ間の依存関係を不必要に増やさない. 循環依存は絶対作らない
- そもそもライブラリ化する必要があるかを検討する
- 他パッケージの msgファイルから生成されたヘッダファイルを includeしない
- テストしやすいように書く
パッケージ間の依存は少ない方が望ましい. 特に循環参照が発生してしまうと, ビルドができない状態に陥ってしまう.
汎用的なもの以外は切り出さない方がいい. パッケージ間の依存を返って高めてしまう可能性がある. 別パッケージにするほど有用かをまず考える. 単に関数の切り出しなら, 同じパッケージ内にライブラリを 作成した方が良い.
msgファイルから生成されたヘッダファイルを includeすると, ライブラリがその msgを持つ パッケージに依存してしまう. 結果そのライブラリをリンクするパッケージも, その msgを持つ パッケージに依存してしまうことになってしまう.
メッセージとして受け取ったデータの処理をライブラリに切り出したい場合は, メッセージに含まれるデータ型(std::vector等)を渡すようにする. もしくは別途定義したメッセージの型に依存しない class, structに移し替える.
下記のような設計, 実装は推奨されない.
ライブラリ側でグローバル変数で状態を保存するのでなく, classや構造体に状態を持たせた 方がよい(errnoのように真に大域的なグローバル変数は例外であるが, そのような変数が 必要になることはほとんどない).
C++であればメソッドとして実装, C言語であれば引数でポインタを渡すように実装すればよい.
またグローバル変数を使う関数の場合は, 複数スレッドを使った場合, 値の取得, 更新に おいて問題が発生する可能性がある(スレッドセーフ). ROSではユーザの見えないところで スレッドが使われる可能性もあるため, 極力グローバル変数を使わないことが望ましい. 必要な場合は複数スレッドからのアクセスがあるかを考慮し, その可能性がある場合は 排他制御を行う必要性を検討する.
ライブラリ関数の引数がない or 戻り値が void型ということは副作用のために 関数呼び出しを行うことになる. このような関数はテストしづらく, 理解もしづらい. ライブラリ関数(not クラスメソッド)については副作用のない関数型スタイルで あることが望ましい. つまり引数が同一であれば結果が一意になる関数.
現在のライブラリ関数のネーミングには以下の問題がある
- 名前が単純すぎて理解が難しい
- 名前が単純すぎてシンボル名重複が発生しうる
例えば fusionライブラリには init, destroyと名前付けされた関数が存在するが,
これだけ見ると何に関する初期化, 終了処理なのかがわからない. また名前が
ありふれているためシンボル名が重複してしまう可能性も高い.
- ライブラリの prefixを使う
- namespaceを使う
上記で挙げた fusionライブラリの場合, fusion_init, fusion_destroyとする.
こうすることで少なくとも fusionに関する処理ということはわかる.
全体を namespace autoware::fusion {}(もしくは autoware)でラップする. 利用するときは
autoware::fusion::init, autoware::fusion_init等になる. システム全体が namespaceを
使っているのであればこの方法が望ましいと思う(特に Autowareのパッケージを一部だけ
切り出し, 別プロジェクトで使うというケースがある場合).
exportするシンボルを明確にする. 現在の実装では不必要に多くのシンボルが export されている. 他パッケージから参照しない関数は exportしないようにする.
C++言語, 無名 namespaceでラップする(ファイルローカルの static指定は非推奨). class, structであれば privateにする. C言語では staticを指定し, スコープをファイルローカルにする.
C++11(14, 17)には型推論等便利な機能が多く加わっているので, C++11以降の内容が 記載されている本を一読することを推奨する.