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

バグを修正 #3

Closed
4 of 5 tasks
github-actions bot opened this issue Nov 30, 2022 · 5 comments · Fixed by #14
Closed
4 of 5 tasks

バグを修正 #3

github-actions bot opened this issue Nov 30, 2022 · 5 comments · Fixed by #14
Labels
初級 全ての応募者の必須課題です。
Milestone

Comments

@github-actions
Copy link

github-actions bot commented Nov 30, 2022

本プロジェクトには、至る所にバグが潜んでいます。下記のリストを参考にバグを修正してください。

  • レイアウトエラー
  • メモリリーク
  • パースエラー
  • 例外の処理漏れ
  • クラッシュ
@github-actions github-actions bot added the 初級 全ての応募者の必須課題です。 label Nov 30, 2022
@github-actions github-actions bot added this to the 課題 milestone Nov 30, 2022
@kokoichi206
Copy link
Owner

kokoichi206 commented Dec 1, 2022

API の call について

今回使用している io.ktor.client.request パッケージの HttpClient.get について、以下の記載がある。
この例外の処理が漏れている。

Executes a HttpClient GET request, with the specified url as URL and an optional block receiving an
HttpRequestBuilder for further configuring the request.
Tries to receive a specific type T, if fails, an exception is thrown.

例えばホスト名が間違ってる時などは java.net.UnknownHostException などが返ってきてアプリがクラッシュする。

今呼んでいる API のレスポンスコードとして考えられるものはこちらに記載がある

  • 422: パラメーター間違いなど(query がない状態もこれに該当する!)
  • 503: サーバー側で利用できない状態

特に現状、エラーを補足してユーザーに表示するとかもないので、とりあえずキャッチだけしてクラッシュしないようにする。

クエリパラメーターがない場合について

現状、入力がない場合でも API を呼ぶようになっている。
(この時 https://api.github.com/search/repositories?q= の URL にアクセスされる)

この時 localizedMessage には以下のメッセージが表示される。

 Client request(https://api.github.com/search/repositories?q=) 
invalid: 422 Unprocessable Entity. Text: "{"message":"Validation Failed","errors":
[{"resource":"Search","field":"q","code":"missing"}],"documentation_url":"https://docs.github.com/v3/search"}"

このエラーに関しては、以下理由から API call 前に弾きたい

  • エラーというほどでもなくない?
    • エラーとしてログに出すような情報ではない
  • ユーザーのミスなので、API を呼ばない方がいい
    • (サーバー側からしたら 4xx を返しているので)

チェックが怠る可能性があるため、API を呼び出す関数内でバリデーションをかけることにする。

@kokoichi206
Copy link
Owner

kokoichi206 commented Dec 1, 2022

ViewModel と Context について

ViewModel の概要 に以下の記載がある。

ViewModel は ViewModelStoreOwner よりも長く存続する可能性があるため、
メモリリークを防ぐためにライフサイクル関連の API(Context や Resources など)の参照を保持しないようにします。

Context を意識した AndroidViewModel があるので、そちらを使う。

Application context aware ViewModel.
Subclasses must have a constructor which accepts Application as the only parameter.

@kokoichi206
Copy link
Owner

kokoichi206 commented Dec 1, 2022

Fragment での View-Binding について

このコミット_binding を削除したけど、これよくなかったみたい。
(Fragment のライフサイクルと View のライフサイクルが別なので、メモリーリークを起こす)

View-Binding#fragments でお勧めされてる方法に従い、実装を戻す。

private var _binding: ResultProfileBinding? = null
// This property is only valid between onCreateView and
// onDestroyView.
private val binding get() = _binding!!

公式では、onCreateView でバインドしたのに対応させて、onDestroyView で解放してあげている。

今回は onViewCreated でバインドしてるので、どこで解放してあげるべきか?

onViewCreated より

Called immediately after onCreateView has returned, but before any saved state has been restored in to the view.

タイミング的には onCreateView と同じと思って良い。
onDestroyView で良さそう。

疑問

これだと何がダメなんだっけ?

class DetailFragment : Fragment(R.layout.fragment_detail) {
    ...
    private var binding: FragmentDetailBinding? = null

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)

        binding = FragmentDetailBinding.bind(view)
        ...
    }

    override fun onDestroyView() {
        super.onDestroyView()
        binding = null
    }
}

@kokoichi206
Copy link
Owner

パースエラー に関しては、このコミットのことかと思われる。

@kokoichi206
Copy link
Owner

kokoichi206 commented Dec 1, 2022

レイアウトエラー

うーん、どこにエラーがあるんだろう。。。

横向きレイアウトで気になった点

端末の横幅によっては、横画面で回転させた時に、詳細画面に情報が表示されない。

Screenshot 2022-12-02 at 7 51 20

バグではない気がするが、今回はこれを解決することとする。

解決方法

横向き専用の画面を用意し、そちらで別レイアウト(左右に分けるとか)にしてもいいが、それでもなお『小さい端末 + 縦向き時』に情報が見切れる恐れがある。

そのどちらも解消する手段として、全体をスクロール可能な領域にする。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
初級 全ての応募者の必須課題です。
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant