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

undef でデフォルト設定を実現しているやつをどうにかする #571

Open
sksat opened this issue May 10, 2023 · 2 comments
Labels
priority::high priorityg high

Comments

@sksat
Copy link
Collaborator

sksat commented May 10, 2023

概要

こういうやつ:

#undef OBCT_STEP_IN_MSEC
#undef OBCT_STEPS_PER_CYCLE
#undef OBCT_CYCLES_PER_SEC
#undef OBCT_MAX_CYCLE
#define OBCT_STEP_IN_MSEC (1)
#define OBCT_STEPS_PER_CYCLE (100)
#define OBCT_CYCLES_PER_SEC (1000 / OBCT_STEP_IN_MSEC / OBCT_STEPS_PER_CYCLE)
#define OBCT_MAX_CYCLE (0xfffffff0u)

詳細

  • 現状,core のヘッダでデフォルトの設定値を define しておいて,その直後で user の settings のヘッダを include しておき,user 側で undef して書き換える,というのが横行している
  • これは以下の理由でとてもよくないため,どうにかしたい
    • 一般に #undef を多用するのはアンチパターン
    • define によって実現したいのは「定数値」の表現なのに,途中で書き換わっている
      • define してから undef して再 define するまでの間にその値を使ったら容易に壊れる
      • ↑これは「(undef するようなものは)このタイミングで値を使わないこと」といった規約やコメントでの 激しい 主張をしているならば事故を多少防げるが,現状はそうなっているわけでもない(おそらく暗黙知にはなっているだろうけれど)
    • なんのために core -> user の依存関係が発生しているのかが不明瞭
    • 「undef したらオーバーライドできる」というのは一種の分岐なので,同じことを実現するとしても分岐が発生していることが明確になるようにすべき(つまり,ifdef すべき)
    • ↑この分岐(core のデフォルト設定を使うのか,user 定義のものを使うのか)はビルド時の設定として表現すべき

close条件

undef が全部ないし問題ない程度に消えるなどしてどうにかなったら

@sksat sksat added the invalid This doesn't seem right label May 10, 2023
@sksat
Copy link
Collaborator Author

sksat commented May 10, 2023

それはそれとして,c2a-core のインターフェース定義をするヘッダと,c2a-core が設定値のデフォルト値を提供するヘッダは分割したい

@sksat
Copy link
Collaborator Author

sksat commented May 10, 2023

c2a-core/Settings/default_hoge.h みたいなかんじになるのかな

@meltingrabbit meltingrabbit added priority::high priorityg high and removed invalid This doesn't seem right labels May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority::high priorityg high
Projects
None yet
Development

No branches or pull requests

2 participants