gen0083 / android-engineer-codechechek

https://gen0083.github.io/android-engineer-codechechek/
Apache License 2.0
0 stars 0 forks source link

SearchContentのUIにビジネスロジックが表出してしまっているのを修正したい #103

Closed gen0083 closed 3 weeks ago

gen0083 commented 3 weeks ago

検索イベントをdebounceで頻度を少なくしているけど、これってビジネスロジックじゃない? TextFieldStateをState hoistingでScreenModelへ持っていけば、このあたりの問題も解決するんじゃなかろうか

ただしそうすると、SearchTextFieldがScreenModelに依存することになるので、interfaceでScreenModelに依存しないようにしたほうがいいだろう とりあえずState hoistingするだけやってみよう

gen0083 commented 3 weeks ago

debounceでテキストの変更から一定時間経ったら検索をトリガーするのはビジネスロジックか? UIロジックじゃないの? TextFieldStateがいろいろ機能を持ちすぎてる気がするのが問題な気もするなあ・・・

State hoistingをやってみているが、今の状態のほうが適切な気もする・・・ ScreenModelで直接rememberTextFieldState()は呼べない(Composable関数内でしか呼べない) (ただこれは、ViewModelでも同じことなのでVoyagerの問題というわけではないが)

それにState hoistingすることによって、次の2つを成立させるのが難しくなる

現状でもこれはできていないが、ScreenModelで検索文字列を保持して、その文字が同じであればSkipするようにすれば解決できる

gen0083 commented 3 weeks ago

ViewModelに持っていくなら直接コンストラクタを呼び出すのでいいのか・・・

https://www.droidcon.com/2023/11/21/basictextfield2-a-textfield-of-dreams-1-2/

gen0083 commented 3 weeks ago

ViewModelに持っていく場合、複数の状態を公開するようになってしまう(というかそれが前提みたいな書き方な気がする)

VoyagerはStateが1つだけなので、ScreenModelのStateとしてはtextFieldStateを公開できない ただ直接参照すればいいだけだが・・・

gen0083 commented 3 weeks ago

ビジネスロジックかUIロジックかなんてことを議論したいんじゃなくて(というかどっちになるか正解はないように思う)、UIにロジックがあるのが気持ち悪いから修正しようよが始まりだったように思う ただでさえUIのためのスタイルでComposable関数は読みづらいのに、そこにロジックが交じると余計にカオスになっていくと思う ifによる状態によって表示・非表示が変わる程度ならいいが、今回問題にしてる検索処理を走らせるトリガーなんかはScreenModel(ViewModel)でやったほうが合理的かなあと思う