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

glob importの取り扱いについて考える #467

Closed
3 tasks done
qryxip opened this issue Apr 19, 2023 · 6 comments · Fixed by #708
Closed
3 tasks done

glob importの取り扱いについて考える #467

qryxip opened this issue Apr 19, 2023 · 6 comments · Fixed by #708

Comments

@qryxip
Copy link
Member

qryxip commented Apr 19, 2023

内容

このようなワイルドカードインポートについて、やっているところ(主にqwertyさん)と避けているところ(主に私)が混在しているため、どちらかに統一します。

以前の議論は#135 (comment)という認識です。

Rustでは名前空間を何でもかんでもフラットにするような文化はそんなになく、別に止められているわけではないけど積極的に推進されてもいないはずです(ただし「外」から見える名前空間をフラットにするために子モジュール下から親モジュール下にre-exportされるといったことはよく行われます)。そのため私が書いた部分はqwertyさんのスタイルから外れた書き方をしていました。

ただこのスレッドで書かれている通り、このスタイルは別にアンチパターン認定されているわけではありませんし、SuitCaseさんも同意しています。そのため#466とは逆に私が書いた部分をqwertyさんに合わせるのが丸いのかなと思っています。Rustのuseは結構柔軟によしなに可視性を解決してくれるので、コードを読む分はともかく書く分には支障は出ないかと思います。
Rustのモジュールを詳細に理解する(5) 可視性 - 簡潔なQ (「可視性と use」を参照のこと)

Pros 良くなる点

  • このリポジトリにPRしに来た人が混乱しなくてよくなる

Cons 悪くなる点

実現方法

VOICEVOXのバージョン

N/A

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux

その他

@Hiroshiba
Copy link
Member

統一は賛成です! どっちに統一するかですよね。

プログラミングの一般的な観点から言うと、*でインポートは便利だけどインポート順序で読み込まれる関数が変わるなどの危険もあるだろうなぁと思ってます。
Pythonなどではたしか*は非推奨なのですが、Rustは特に無い感じでしょうか。

であれば、何か問題が起きるまでは*に統一するのも良さそうに思います…!

@qryxip qryxip changed the title ワイルドカードインポートの取り扱いについて考える glob importの取り扱いについて考える Apr 19, 2023
@qryxip
Copy link
Member Author

qryxip commented Apr 19, 2023

Pythonなどではたしか*は非推奨なのですが、Rustは特に無い感じでしょうか。

まあ非推奨ってことはなく、局所的に使う分にはむしろ一般的なんですが、qwertyさんスタイルレベルのはそんなに無い気がするのでそれでissueにすべきか今まで悩んでました。

インポート順序で読み込まれる関数が変わるなどの危険

Rustだとその場合「どっちかを名指ししてインポートしろ」というようなコンパイルエラーになったような...? ただしこういう場合はa::Sが優先されるといったことはあったはず。

use a::S;
use b::*; // 中に`S`が入っている

どっちかというと、有り得るデメリットとしてはリーダビリティの低下かと思ってます。

@Hiroshiba
Copy link
Member

なるほどです。

難しい判断ですが、特に指標がないのであれば前に習うのが丸いのかなと思いました!!
なので最初に @qryxip さんが提案されていた形の、qryxipさんが書いたとこ周りを*にするのが良さそうかかと!!

@qryxip
Copy link
Member Author

qryxip commented Jul 30, 2023

この件、最近考えていることがあります。

use super::*って(stdを含む)dependencyのuseも一緒に持って来るんですよね。「下」でしか使わないアイテムを「上」でuseしてもいいわけですし、「上」と「下」の両方でしてもよいです。例えば次のコードにおいて、mod b下の方のuse std::marker::PhantomDataは無くても動きますが、rustcはこれについて何の警告もしません(なぜならglob importより非glob importの方が優先されるという明確な規則があるため)。

mod a {
    use std::marker::PhantomData;

    type _A = PhantomData<()>;

    mod b {
        use std::marker::PhantomData;

        use super::*;

        type _B = _A;
        type _C = PhantomData<()>;
    }
}

既にこのリポジトリにそういうのがあったような記憶があります。

名前空間をフラットにすると考えるならdependencyのuseっていっそのこと一番上(lib.rs)に持って来るべきなんじゃないか?と思ったりもするのですが、そのようにしているコードって今まで見たことが無いんですよね。

まあこういうわけで、use super::*;の方に合わせるとは発言したわけですが、やはり未だにちょっと引っ掛かってます。

@Hiroshiba
Copy link
Member

Hiroshiba commented Jul 30, 2023

use super::*;のほうが少人数での開発速度は上がると思います!
でも保守や新規参入者の学習曲線を考えると、直接指定したimportのが良いと思います。

なのでAPIなどが全部固まって開発が完了したあとに、1つ1つのimportに切り替えるのが良いのかなと。
ただまあこうすると新しいのの開発速度は下がるのと、いつ開発が完了したと判断するのかよくわからないので難しいですね・・・。
とりあえずこのまま共存し続けて、もう新しい開発はほぼ無さそうだというタイミングで再考するのはどうでしょうか。
例えばVVM-async化完了のタイミングとか。

(あと、*でのimportと直接のimportが被る件については、既存開発者がなんか変だなと感じる以外は意外とデメリット無いなぁと思いました。
新しい開発のために*でのimportは一旦そのままにして、新規参入者のために直接指定するimportを増やしていっても良いかも・・・・・・・・・・?)

@qryxip
Copy link
Member Author

qryxip commented Jul 31, 2023

新規参入

これは本当に重視したいところですね...なんとかしてバザール型にしたいという思いがあります。

とかもそれを願ってのものでした。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants