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

ユーザー画面 #39

Merged
merged 5 commits into from
Apr 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions qiita_app/lib/pages/follower_following_list_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import 'package:qiita_app/widgets/follow_container.dart';

class FollowerFollowingListPage extends StatefulWidget {
final String listType;
final String userId;

const FollowerFollowingListPage({
Key? key,
required this.listType,
required this.userId,
}) : super(key: key);

@override
Expand Down Expand Up @@ -66,11 +68,9 @@ class _FollowerFollowingListPageState extends State<FollowerFollowingListPage> {

List<User> fetchedUsers;
if (widget.listType == 'following') {
fetchedUsers =
await QiitaRepository.fetchFollowingUsers(authenticatedUser.id);
fetchedUsers = await QiitaRepository.fetchFollowingUsers(widget.userId);
} else {
fetchedUsers =
await QiitaRepository.fetchFollowersUsers(authenticatedUser.id);
fetchedUsers = await QiitaRepository.fetchFollowersUsers(widget.userId);
}
Comment on lines 70 to 74
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本当に文字列listTypeがfollowing以外だった場合、全てfollowerとなるでしょうか?
この場合、文字列'hoge'のようなテキトーな文字列でもfollowing以外と判定され、follwer扱いになってしまいます。

それを防ぐために、以下のようにenumを活用してみるといいかもしれません。

enum FollowerFollowingListType {
  follower,
  following;
}

class FollowerFollowingListPage extends StatefulWidget {
  final FollowerFollowingListType listType;
  final String userId;

// 中略

      List<User> fetchedUsers = [];
      if (widget.listType == FollowerFollowingListType.following) {
        fetchedUsers = await QiitaRepository.fetchFollowingUsers(widget.userId);
      } else if (widget.listType == FollowerFollowingListType.follower) {
        fetchedUsers = await QiitaRepository.fetchFollowersUsers(widget.userId);
      }

String型だと任意の文字列をFollowerFollowingListPageクラスに渡すことができますが、新しく定義したFollowerFollowingListType型はfollowerとfollowingしか存在しないため、先ほどの'hoge'のような異物を弾くことができます。
異物を弾くことができるので、`followor'みたいなスペルミスも防ぐことができます。

debugPrint('${fetchedUsers.length} users loaded successfully');

Expand Down
171 changes: 171 additions & 0 deletions qiita_app/lib/pages/user_page.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import 'package:flutter/material.dart';
import 'package:qiita_app/models/article.dart';
import 'package:qiita_app/models/user.dart';
import 'package:qiita_app/repository/qiita_repository.dart';
import 'package:qiita_app/widgets/app_title.dart';
import 'package:qiita_app/widgets/article_container.dart';
import 'package:qiita_app/widgets/network_error.dart';
import 'package:qiita_app/widgets/section_divider.dart';
import 'package:qiita_app/widgets/user_info_container.dart';

class UserPage extends StatefulWidget {
final String userId;

const UserPage({required this.userId, Key? key}) : super(key: key);

@override
State<UserPage> createState() => _UserPageState();
}

class _UserPageState extends State<UserPage> {
List<Article> articles = [];
User? loggedInUser;
bool isLoading = true;
bool hasNetworkError = false;
bool isFetching = false;
int currentPage = 1;
late ScrollController _scrollController;
Comment on lines +20 to +27
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ローディングの状態管理で使われる変数がisLoadingisFetchingの2種類定義されてしまっています。
今回修正の必要はありませんが、混乱やバグの原因になるので、今後は1つの役割に対して変数を複数定義しないようにしてください。


@override
void initState() {
super.initState();
_scrollController = ScrollController();
fetchUserInfo();
_scrollController.addListener(_scrollListener);
}

Future<void> fetchUserInfo() async {
setState(() {
isLoading = true;
});

try {
User userInfo = await QiitaRepository.fetchUserInfo(widget.userId);
List<Article> userArticles = await QiitaRepository.fetchUserArticles(
widget.userId,
page: currentPage);

if (mounted) {
setState(() {
loggedInUser = userInfo;
articles = userArticles;
isLoading = false;
hasNetworkError = false;
});
}
} catch (e) {
debugPrint('Failed to fetch user info: $e');
if (mounted) {
setState(() {
loggedInUser = null;
isLoading = false;
hasNetworkError = true;
});
}
}
}
Comment on lines +37 to +66
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

僕だったらこのfetchUserInfoというメソッドを以下のように実装するかなと思います。

  Future<void> fetchUserInfo() async {
    if (isLoading) return;
    setState(() {
      isLoading = true;
      hasNetworkError = false;
    });
    User? user;
    List<Article> userArticles = [];
    try {
      user = await QiitaRepository.fetchUserInfo(widget.userId);
      userArticles = await QiitaRepository.fetchUserArticles(
        widget.userId,
        page: currentPage,
      );
    } catch (e) {
      debugPrint('Failed to fetch user info: $e');
      setState(() {
        loggedInUser = null;
        hasNetworkError = true;
      });
    } finally {
      setState(() {
        isLoading = false;
      });
    }
    setState(() {
      loggedInUser = user;
      articles = userArticles;
    });
  }

ポイントは以下の4点です。

1. try文の中はシンプルに

#38 (comment) 参照

2. 早期リターンを使う

#38 (comment) 参照
ローディング中にもかかわらず、fetchUserInfoメソッドが実行されるのを防ぐために1番最初に if (isLoading) return;という処理を記述しています。

3. 変数名をuserInfoからuserに変更

変数名がuserInfoだと、ユーザーと記事の両方の値を格納しているのか誤解を与えてしまう恐れがあります。
fetchUserInfoで取得したユーザーを格納する変数なのでuserにしました。

4. try-catch-finally文のfinally句を使う

try-catch文の後ろにfinally句が追加されたtry-catch-finally文という構文がDartを始めとしたプログラミング言語には用意されています。
finally内に書いた処理は、try内で生じたエラーの有無にかかわらず、最終的に必ず実行されます。
今回の例だと、ユーザー情報の取得に成功しても、失敗しても、最終的にisLoadingをfalseにする実装が必要になります。
これをそれぞれ別の場所に書くのではなく、finally文にまとめて書くことにより、コードの行数を削減したり、修正が用意になったりします。


void _scrollListener() {
if (!isFetching &&
_scrollController.position.pixels ==
_scrollController.position.maxScrollExtent) {
currentPage++;
fetchUserArticles();
}
}

Future<void> fetchUserArticles() async {
setState(() {
Comment on lines +76 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もう少し、記事の追加読み込みを行なっていることがわかるようなメソッド名の方が良いのかなと思いました。
処理の中身に関しては、fetchUserInfoメソッドと同じことが言えます。

isFetching = true;
});

try {
List<Article> additionalArticles =
await QiitaRepository.fetchUserArticles(widget.userId,
page: currentPage);

if (mounted) {
setState(() {
articles.addAll(additionalArticles);
isFetching = false;
});
}
} catch (e) {
debugPrint('Failed to fetch additional user articles: $e');
if (mounted) {
setState(() {
isFetching = false;
});
}
}
}

@override
Widget build(BuildContext context) {
if (isLoading) {
return const Scaffold(
appBar: AppTitle(
title: 'UserPage',
showBottomDivider: true,
),
body: Center(child: CircularProgressIndicator()),
);
}
if (hasNetworkError) {
return Scaffold(
appBar: const AppTitle(title: 'UserPage', showBottomDivider: true),
body: NetworkError(
onPressReload: () {
setState(() {
hasNetworkError = false;
});
fetchUserInfo();
},
),
);
}
return Scaffold(
appBar: const AppTitle(
title: 'UserPage',
showBottomDivider: true,
showReturnIcon: true,
),
Comment on lines +103 to +132
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1つの画面にScafolldが3つあり、AppTitleもほぼ同じ内容なので、条件分岐を工夫すればもっと上手く共通化できるかもしれませんね。
今回修正の必要はないです。

body: Column(
children: [
if (loggedInUser != null) UserInfoContainer(user: loggedInUser!),
const SectionDivider(text: '投稿記事'),
Expanded(
child: ListView.builder(
itemCount: articles.length + 1, // +1 for loading indicator
itemBuilder: (context, index) {
if (index < articles.length) {
return ArticleContainer(
article: articles[index],
showAvatar: false,
);
} else {
return _buildLoadMoreIndicator();
}
},
controller: _scrollController,
),
),
],
),
);
}

Widget _buildLoadMoreIndicator() {
return Center(
child: isFetching
? const CircularProgressIndicator()
: ElevatedButton(
onPressed: () {
currentPage++;
fetchUserArticles();
},
child: const Text('もっと読み込む'),
),
);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

仕様では、ボタンではなくスクロールによるページネーションだったはずです。
動作確認用かもしれませんが、仕様と異なるので修正お願いします🙏

23 changes: 21 additions & 2 deletions qiita_app/lib/repository/qiita_repository.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:convert';

import 'package:flutter/foundation.dart';
import 'package:flutter_dotenv/flutter_dotenv.dart';
import 'package:http/http.dart' as http;
Expand Down Expand Up @@ -189,8 +190,10 @@ class QiitaRepository {
return accessToken;
}

static Future<List<Article>> fetchUserArticles(String userId) async {
final url = Uri.parse('${Urls.qiitaBaseUrl}/users/$userId/items');
static Future<List<Article>> fetchUserArticles(String userId,
{int page = 1}) async {
final url =
Uri.parse('${Urls.qiitaBaseUrl}/users/$userId/items?page=$page');
try {
final response = await http.get(url);

Expand Down Expand Up @@ -242,4 +245,20 @@ class QiitaRepository {
throw Exception('Failed to load followers: $e');
}
}

static Future<User> fetchUserInfo(String userId) async {
final url = Uri.parse('${Urls.qiitaBaseUrl}/users/$userId');
try {
final response = await http.get(url);

if (response.statusCode == 200) {
final Map<String, dynamic> jsonResponse = jsonDecode(response.body);
return User.fromJson(jsonResponse);
} else {
throw Exception(_exceptionMessage(response.statusCode));
}
} catch (e) {
throw Exception('Failed to fetch user info: $e');
}
}
}
9 changes: 9 additions & 0 deletions qiita_app/lib/widgets/follow_container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'package:flutter/material.dart';
import 'package:qiita_app/constants/app_colors.dart';
import 'package:qiita_app/constants/app_text_style.dart';
import 'package:qiita_app/models/user.dart';
import 'package:qiita_app/pages/user_page.dart';

class FollowContainer extends StatelessWidget {
final User user;
Expand All @@ -14,6 +15,14 @@ class FollowContainer extends StatelessWidget {
@override
Widget build(BuildContext context) {
return GestureDetector(
onTap: () {
Navigator.push(
context,
MaterialPageRoute(
builder: (context) => UserPage(userId: user.id),
),
);
},
child: Container(
margin: const EdgeInsets.symmetric(horizontal: 20, vertical: 8),
padding: const EdgeInsets.symmetric(horizontal: 20, vertical: 8),
Expand Down
8 changes: 4 additions & 4 deletions qiita_app/lib/widgets/user_info_container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ class _UserInfoContainerState extends State<UserInfoContainer> {
Navigator.push(
context,
MaterialPageRoute(
builder: (context) =>
const FollowerFollowingListPage(
builder: (context) => FollowerFollowingListPage(
listType: 'following',
userId: widget.user.id, // ユーザーIDを渡す
),
),
);
Expand Down Expand Up @@ -88,9 +88,9 @@ class _UserInfoContainerState extends State<UserInfoContainer> {
Navigator.push(
context,
MaterialPageRoute(
builder: (context) =>
const FollowerFollowingListPage(
builder: (context) => FollowerFollowingListPage(
listType: 'followers',
userId: widget.user.id, // ユーザーIDを渡す
),
),
);
Expand Down
Loading