Goal
ページネーションをリファクタリングする。
Dev-Environment
OS: Windows8.1
Erlang: Eshell V6.4, OTP-Version 17.5
Elixir: v1.0.4
Phoenix Framework: v0.13.1
PostgreSQL: postgres (PostgreSQL) 9.4.4
Erlang: Eshell V6.4, OTP-Version 17.5
Elixir: v1.0.4
Phoenix Framework: v0.13.1
PostgreSQL: postgres (PostgreSQL) 9.4.4
Wait a minute
ソースコードが汚い、汚い！！と騒いでいた時期がありましたね(笑)
まずは、私の中で一番目に付くページネーションの部分をリファクタリングします。
内容的にリファクタリングと言えるのか分かりませんが・・・
私の中では、ソースコードの改善 = リファクタリングだと思っています。
細かいことは、まぁいいでしょう。
私の中では、ソースコードの改善 = リファクタリングだと思っています。
細かいことは、まぁいいでしょう。
重要なのは、読みやすいか読み辛いかですね。
Index
Paginate refactoring
|> User List of header layout
|> User profile of header layout
|> Duplicate processing in the action
|> Add function of pagination
|> Fix mistakes
|> User List of header layout
|> User profile of header layout
|> Duplicate processing in the action
|> Add function of pagination
|> Fix mistakes
User List of header layout
レイアウトのヘッダー部分にあるユーザ一覧リンクを修正します。
ファイル: web/templates/layout/header.html.eex
ヘッダーのリンク部分を修正します。
ヘッダーのリンク部分を修正します。
修正前
<li><%= link "All Users", to: add_first_page_param(user_path(@conn, :index)) %><li>
修正後
<li><%= link "All Users", to: user_path(@conn, :index) %><li>
ファイル: web/controllers/user_controller.ex
送信されるパラメータに値がなくても動作するように修正します。
送信されるパラメータに値がなくても動作するように修正します。
以下の行を削除して下さい。
plug :scrub_params, "select_page" when action in [:index, :show]
修正前
def index(conn, %{"select_page" => select_page}) do
page = SampleApp.User.paginate(select_page)
...
end
修正後
def index(conn, params) do
select_page = params["select_page"]
if is_nil(select_page) do
select_page = "1"
end
page = SampleApp.User.paginate(select_page)
...
end
この時点では、あまりよろしくない方法を取っています。
ですが、安心して下さい。
この記事の後ろの方で重複する処理を再度修正します。
この記事の後ろの方で重複する処理を再度修正します。
User profile of header layout
同じく、レイアウトのヘッダー部分にあるユーザプロファイルへのリンクを修正します。
修正する量が少し多いので、ミスに注意して下さい。
ファイル: web/templates/layout/header.html.eex
ヘッダーのリンク部分を修正します。
ヘッダーのリンク部分を修正します。
修正前
<li><%= link "Profile", to: add_first_page_param(user_path(@conn, :show, current_user(@conn).id)) %><li>
修正後
<li><%= link "Profile", to: user_path(@conn, :show, current_user(@conn)) %><li>
ファイル: web/controllers/user_controller.ex
送信されるパラメータに値がなくても動作するように修正します。
送信されるパラメータに値がなくても動作するように修正します。
修正前
def show(conn, %{"id" => id, "select_page" => select_page}) do
user = Repo.get(SampleApp.User, id) |> Repo.preload(:relationships) |> Repo.preload(:reverse_relationships)
ids_list = list_map_to_value_list(user.followed_users, :followed_id)
page = SampleApp.Micropost.paginate(user.id, select_page, ids_list)
changeset = SampleApp.Micropost.new_changeset(
%SampleApp.Micropost{}, %{content: "", user_id: user.id})
...
end
修正後
def show(conn, params) do
select_page = params["select_page"]
id = params["id"]
if is_nil(select_page) do
select_page = "1"
end
user = Repo.get(SampleApp.User, id) |> Repo.preload(:relationships) |> Repo.preload(:reverse_relationships)
ids_list = list_map_to_value_list(user.followed_users, :followed_id)
page = SampleApp.Micropost.paginate(user.id, select_page, ids_list)
changeset = SampleApp.Micropost.new_changeset(
%SampleApp.Micropost{}, %{content: "", user_id: user.id})
...
end
ファイル: web/templates/user/show_follow.html.eex
上記以外にもリンクを作っているところがあります。
同様の修正を行います。
上記以外にもリンクを作っているところがあります。
同様の修正を行います。
修正前
<div><%= link "view my profile", to: add_first_page_param(user_path(@conn, :show, @user)), class: "btn btn-default btn-xs" %></div>
修正後
<div><%= link "view my profile", to: user_path(@conn, :show, @user), class: "btn btn-default btn-xs" %></div>
修正前
<div style="clear: left; margin-top: 30px;">
<%= if @users do %>
<div class="user_avatars">
<%= for follow_user <- @users do %>
<a href="<%= add_first_page_param(user_path(@conn, :show, follow_user)) %>">
<img src="<%= get_gravatar_url(follow_user) %>" class="gravatar">
</a>
<% end %>
</div>
<% end %>
</div>
修正後
<div style="clear: left; margin-top: 30px;">
<%= if @users do %>
<div class="user_avatars">
<%= for follow_user <- @users do %>
<a href="<%= user_path(@conn, :show, follow_user) %>"> <!-- 修正部分 -->
<img src="<%= get_gravatar_url(follow_user) %>" class="gravatar">
</a>
<% end %>
</div>
<% end %>
</div>
修正前
<div style="clear: right; margin-left: 250px;">
<%= if @users do %>
<ul>
<%= for follow_user <- @users do %>
<li>
<a href="<%= add_first_page_param(user_path(@conn, :show, follow_user)) %>">
<img src="<%= get_gravatar_url(follow_user) %>" class="gravatar">
<h4><%= follow_user.name %></h4>
</a>
</li>
<% end %>
</ul>
...
<% end %>
</div>
修正後
<div style="clear: right; margin-left: 250px;">
<%= if @users do %>
<ul>
<%= for follow_user <- @users do %>
<li>
<a href="<%= user_path(@conn, :show, follow_user) %>"> <!-- 修正部分 -->
<img src="<%= get_gravatar_url(follow_user) %>" class="gravatar">
<h4><%= follow_user.name %></h4>
</a>
</li>
<% end %>
</ul>
...
<% end %>
</div>
ファイル: web/templates/user/index.html.eex
上記以外にもリンクを作っているところがあります。
同様の修正を行います。
上記以外にもリンクを作っているところがあります。
同様の修正を行います。
修正前
<a href="<%= add_first_page_param(user_path(@conn, :show, user)) %>">
修正後
<a href="<%= user_path(@conn, :show, user) %>">
ファイル: web/views/user_view.ex
不要になった関数を削除します。
不要になった関数を削除します。
def add_first_page_param(action) do
"#{action}?select_page=1"
end
ファイル: web/views/layout_view.ex
不要になった関数を削除します。
不要になった関数を削除します。
def add_first_page_param(action) do
"#{action}?select_page=1"
end
Duplicate processing in the action
Userコントローラのindex、show、following、followersで重複している処理がありますね。
この重複部分を一つにまとめます。
この重複部分を一つにまとめます。
PaginationHelperにて同じような処理をしているのを覚えていますか？
この部分で対応することにしましょう。
この部分で対応することにしましょう。
ファイル: lib/helpers/pagination_helper.ex
ページ遷移できない場合は、問答無用で最初のページ情報を返すように修正します。
ページ遷移できない場合は、問答無用で最初のページ情報を返すように修正します。
以下の定数を追加。
@first_page "1"
修正前
def paginate(query, select_page) do
if is_able_to_paginate?(select_page) do
query |> SampleApp.Repo.paginate(page: select_page, page_size: @page_size)
else
nil
end
end
修正後
def paginate(query, select_page) do
if is_able_to_paginate?(select_page) do
query |> SampleApp.Repo.paginate(page: select_page, page_size: @page_size)
else
query |> SampleApp.Repo.paginate(page: @first_page, page_size: @page_size)
end
end
ファイル: web/controllers/user_controller.ex
index、show、following、followersアクション内にある重複している処理を削除します。
以下の部分を削除します。
index、show、following、followersアクション内にある重複している処理を削除します。
以下の部分を削除します。
if is_nil(select_page) do
select_page = "1"
end
Add function of pagination
show_follow.html.eexでのユーザ表示をするため、
専用のページネーションの関数をUserモデルへ追加します。
専用のページネーションの関数をUserモデルへ追加します。
ファイル: web/models/user.ex
ページネーション用の関数を追加します。
ページネーション用の関数を追加します。
def show_follow_paginate(select_page, ids_list) do
SampleApp.Helpers.PaginationHelper.paginate(
from(u in SampleApp.User, where: u.id in ^ids_list, order_by: [asc: :name]),
select_page)
end
ファイル: web/controllers/user_controller.ex
上記で定義した関数を使うように修正します。
上記で定義した関数を使うように修正します。
修正前
def followers(conn, params) do
...
user = Repo.get(SampleApp.User, id) |> Repo.preload(:relationships) |> Repo.preload(:reverse_relationships)
ids_list = list_map_to_value_list(user.followers, :follower_id)
page = SampleApp.Helpers.PaginationHelper.paginate(
from(u in SampleApp.User, where: u.id in ^ids_list, order_by: [asc: :name]),
select_page)
...
end
修正後
def following(conn, params) do
...
user = Repo.get(SampleApp.User, id) |> Repo.preload(:relationships) |> Repo.preload(:reverse_relationships)
page = SampleApp.User.show_follow_paginate(
select_page, list_map_to_value_list(user.followed_users, :followed_id))
...
end
修正前
def followers(conn, params) do
...
user = Repo.get(SampleApp.User, id) |> Repo.preload(:relationships) |> Repo.preload(:reverse_relationships)
ids_list = list_map_to_value_list(user.followers, :follower_id)
page = SampleApp.Helpers.PaginationHelper.paginate(
from(u in SampleApp.User, where: u.id in ^ids_list, order_by: [asc: :name]),
select_page)
...
end
修正後
def followers(conn, params) do
...
user = Repo.get(SampleApp.User, id) |> Repo.preload(:relationships) |> Repo.preload(:reverse_relationships)
page = SampleApp.User.show_follow_paginate(
select_page, list_map_to_value_list(user.followers, :follower_id))
...
end
Fix mistakes
以前、修正するのを忘れていた部分を見つけました。
ファイル: web/controllers/user_controller.ex
テンプレートへ送る名前を間違っていました。
followed_users: -> users:
テンプレートへ送る名前を間違っていました。
followed_users: -> users:
修正前
def following(conn, params) do
...
if page do
...
else
conn
|> put_flash(:error, "Invalid page number!!")
|> render("following.html", user: user, followed_users: [])
end
end
修正後
def following(conn, params) do
...
if page do
...
else
conn
|> put_flash(:error, "Invalid page number!!")
|> render("following.html", user: user, users: [])
end
end
修正前
def followers(conn, params) do
...
if page do
...
else
conn
|> put_flash(:error, "Invalid page number!!")
|> render("followers.html", user: user, followed_users: [])
end
end
修正後
def followers(conn, params) do
...
if page do
...
else
conn
|> put_flash(:error, "Invalid page number!!")
|> render("followers.html", user: user, users: [])
end
end
Speaking to oneself
少しは読みやすくなったでしょうか？
リファクタリング一回目としては、こんなところにしておきます。
リファクタリング一回目としては、こんなところにしておきます。
次は、ビュー関連のリファクタリングをすることにします。
変更点の一覧については以下のリンク先にあります。
変更点: Github - darui00kara/phoenix_tutorial (Pagination refactoring)
変更点: Github - darui00kara/phoenix_tutorial (Pagination refactoring)
Bibliography
特になし