LoginSignup
11
7
@kentotsuka

未経験の時に作った個人開発アプリの設計・実装を修正した話。

Last updated at Posted at 2023-10-06

はじめに

軽く自己紹介すると、エンジニア歴９ヶ月のひよっこです。
先日、ふと自分が未経験の時に作ったアプリのコードを見返すと、ツッコミどころが盛り沢山だったので、設計、実装を見直し改修しました。
エンジニアになってまだ僅かですが、過去に自分が書いたコードにメスを入れたことで今になってわかる気づきが得られたので記事にしようと思いました。

どんなアプリ？

Youtube上にある #4kwalk, #walkingtour などのバーチャルツアーの動画を活用し、まるで世界中の街中にいるかのような雰囲気を体感できるアプリです。→気になった方はこちら
1.gif
バックエンドをRails API、フロントエンドをVue.jsで作っています。また、地図やYoutubeの動画をアプリから参照できるように、Geocoding API や YouTube Data API などを導入しています。

今回の改修では設計と実装に手を加えてつつも、仕様は変えないようにしました。

詳細

上のGIF動画でもあるように、作成したアプリには地図上のスポットをクリックしたと同時にYoutubeで取得した動画を一覧を表示する機能があります。手を加えたのは、この機能に関わる設計、実装の部分です。

設計について

問題部分

設計、詳細にいうとDB設計ですが元は以下のようになってました。

image.png

赤が地図上のピンを表す地点テーブル、青がYoutube動画情報を格納する動画テーブルとなります。
何かおかしいことに気づきませんか.....？

そうです...地点テーブルと動画テーブルが紐づいていないのです！

本来であれば、地点テーブルと動画テーブルの関係性は1対多になるはずです。しかし、紐付けすらされておらず動画テーブルは独立したテーブルとなっていました。

  • 本来の形
    image.png

なぜ、このような設計になっていたのか？
開発者本人ですが、すっぽり記憶が抜けて落ちてしまいはっきりとした理由はわかりませんでした；
ただこのような設計により、API側で動画を取得できるようにするためには、地点名をクエリパラメータで指定しなければならず、以下のような不格好なURLになってしまってました。

  • クエリパラメータを含めたURL 
    api/v1/videos?spot_name='ロサンゼルス'

従って、動画の取得も以下のような処理になっています。

  • 動画の取得 
    Video.where(spot: params[:spot_name])

改修内容

地点テーブルと動画テーブルを1対多になるよう紐付けます。
加えて、動画テーブルからカラムarea（国名）、spot（地点名）を削除します。

image.png
また、動画一覧を取得するAPIのURLもルーティングの設定により変更します。

  • 変更後のURL 
    /api/v1/spots/{spot_id}/videos

これにより、クエリパラメータに値を含めなくともAPI側でパスパラメータのspot_idから動画を取得できるような実装に変更できました！

  • 動画の取得 
    Video.where(spot_id: params[:spot_id])

設計の気づき
テーブル設計で最適な関連付けが行われていないことで、ルーティングの定義やデータの取得処理に悪影響を及ぼし、分かりづらい実装になってしまうことがある。

実装について

問題部分

一覧で表示する動画を取得する処理に関連する部分は次のとおりです。

app/controllers/api/v1/videos_controller.rb
class Api::V1::VideosController < ApiController
  include YoutubeApi

  # 一覧で表示する動画を取得
  def index
    # スポットに関する動画を取得
    @search_results = Video.where(spot: params[:spot_name])
    @response = []

    if saved_and_recent?
      @search_results.each do |result|
        @response << { video_id: result.video_id, title: result.title, thumbnail: result.thumbnail, view_count: result.view_count, published_at: result.published_at.strftime("%Y/%m/%d") }
      end
    elsif saved_and_old?
      begin
        # Youtube APIから動画一覧を取得
        get_videos(params, params[:iso])
        # 取得した動画一覧をDBに更新保存
        update_videos(@api_responses, @search_results)
        # 取得した動画一覧をレスポンスで返す
        set_videos
      rescue
        @response = { error: ResourceNotFound }
      end
    else
      begin
        # Youtube APIから動画一覧を取得
        get_videos(params, params[:iso])
        # 取得した動画をDBに保存
        save_videos(@api_responses, params)
        # 取得した動画一覧をレスポンスで返す
        set_videos
      rescue
        @response = { error: ResourceNotFound }
      end
    end

    render json: @response.sort{|x, y| x[:view_count]  <=> y[:view_count] }.reverse
  end

  private

  # 地点に関する動画を全て取得
  def set_videos
    @search_results = Video.where(spot: params[:spot_name])
    # 取得した動画一覧をレスポンスで返す
    @search_results.each do |result|
      @response << { video_id: result.video_id, title: result.title, thumbnail: result.thumbnail, view_count: result.view_count, published_at: result.published_at.strftime("%Y/%m/%d") }
    end
  end

  # 地点に関する動画をDBから取得できる & 動画が最近更新されている
  def saved_and_recent?
    @search_results.present? && @search_results.first.recently?
  end

  # 地点に関する動画をDBから取得できる & 動画が最近更新されていない
  def saved_and_old?
    @search_results.present? && !@search_results.first.recently?
  end
end
app/models/youtube_api.rb
module YoutubeApi
  require 'google/apis/youtube_v3'

  # Youtube
  def get_videos(query, region, after: 2.years.ago, before: Time.now)
    youtube = Google::Apis::YoutubeV3::YouTubeService.new
    youtube.key = Rails.application.credentials.google[:youtube_api_key]
    @api_responses = []

    # 検索結果を取得
    search_results = youtube.list_searches(
      :snippet,
      type: "video",
      q: query[:spot_name_ens] + " " +"walking tour 4k",
      max_results: 24,
      region_code: region,
      # HD 動画のみ
      video_definition: "high",
      # 20 分を超える動画のみ
      video_duration: "long",
      # 埋め込み可能な動画のみを検索
      video_embeddable: true,
      # 関連度が高い順
      order: "relevance",
      # ２年前より後のみ
      published_after: after.iso8601,
      # 本日より前のみ
      published_before: before.iso8601,
      fields: 'items(id(videoId), snippet(title, publishedAt,thumbnails(medium(url))))'
    )

    # 各動画の再生回数を取得（list_searchesメソッドでは再生回数を取得できないため）
    search_results.items.each_with_index do |item, index|
      video_id = search_results.items[index].id.video_id
      video_results = youtube.list_videos(
        :statistics,
        id: video_id,
        max_results: 24,
        fields: 'items(statistics(view_count))'
      )

      view_count = video_results.items[0].statistics.view_count
      snippet = item.snippet
      thumbnail = snippet.thumbnails.medium.url
      # { video_id・動画タイトル・サムネ・再生回数 } を返す
      @api_responses << { video_id: video_id, title: snippet.title, thumbnail: thumbnail, view_count: view_count, published_at: snippet.published_at }
      @api_responses
    end
  end

  # Youtube動画を保存
  def save_videos(results, query)
    results.each do |result|
      Video.create!(
        area: query[:area_name],
        spot: query[:spot_name],
        video_id: result[:video_id],
        title: result[:title],
        thumbnail: result[:thumbnail],
        view_count: result[:view_count],
        published_at: result[:published_at]
      )
    end
  end

  # DBにあるYoutube動画を更新保存
  def update_videos(latest_results, past_results)
    past_results.each_with_index do |result, index|
      result.update!(
        video_id: latest_results[index][:video_id],
        title: latest_results[index][:title],
        thumbnail: latest_results[index][:thumbnail],
        view_count: latest_results[index][:view_count],
        updated_at: Time.current
      )
    end
  end
end

コード量が多くて何してるのかよくわからないと思いますが、一覧で表示する動画は以下のような条件で取得しています。

  • 地点に関する動画がDBに保存されていない場合、Youtubeから動画情報を取得しDBに保存した上で表示
  • 地点に関する動画がDBに保存されている & 直近、動画情報が更新されている場合、DBに保存された動画情報を取得し表示
  • 地点に関する動画がDBに保存されている & 直近、動画情報が更新されていない場合、Youtubeから動画情報を再取得しDBを更新した上で表示

この実装の改善できそうな点について大きく三つ述べます。

改善できそうな点

  1. 冗長なコード
    似たようなコードが書かれた箇所がいくつかあり、コードの可読性は下げてしまっている

  2. 分かりづらいメソッド名
    例えば、 get_videos でDBから取得するのか Youtube API を使って取得するのかコメントで補足しないと明確にわからないようになっている

  3. ロジックの分離が中途半端
    データの取得処理や保存・更新処理などのロジックをモジュールに切り出しているのはいいが、結局のところ条件分岐の処理はコントローラー側に役割を持たせてしまっているので、ロジックの構成がコントローラーとモジュールにまたがってしまっており切り出した意味を成していない

以上の点を踏まえて修正してみました！

app/controllers/api/v1/videos_controller.rb
class Api::V1::VideosController < ApiController
  include Api::Video

  def index
    unless videos.first&.recently_updated?
      fetch_videos
    end

    render json: videos,
           root: 'videos',
           adapter: :json,
           each_serializer: VideoSerializer,
           meta: {
             area: spot.country,
             spot: spot
           }
  end
end
app/controllers/concerns/api/video.rb
module Api::Video
  extend ActiveSupport::Concern
  require 'google/apis/youtube_v3'

  def initialize
    @youtube = Google::Apis::YoutubeV3::YouTubeService.new
    @youtube.key = Rails.application.credentials.google[:youtube_api_key]
  end

  #
  # DBへの保存・更新処理を行う
  #
  def fetch_videos
    videos.present? ? update_videos : save_videos
  end

  #
  # Youtube APIで動画を検索し取得
  #
  def fetch_youtube_videos(after: 2.years.ago, before: Time.now)
    @fetch_youtube_videos ||= @youtube.list_searches(
                                :snippet,
                                type: "video",
                                q: spot.name_ens + " " +"walking tour 4k",
                                max_results: 24,
                                region_code: spot.iso,
                                # HD 動画のみ
                                video_definition: "high",
                                # 20 分を超える動画のみ
                                video_duration: "long",
                                # 埋め込み可能な動画のみを検索
                                video_embeddable: true,
                                # 関連度が高い順
                                order: "relevance",
                                # ２年前より後のみ
                                published_after: after.iso8601,
                                # 本日より前のみ
                                published_before: before.iso8601,
                                fields: 'items(id(videoId), snippet(title, publishedAt,thumbnails(medium(url))))'
                              )
  end

  #
  # Youtube動画の再生回数を取得
  # - list_searchesメソッドでは再生回数を取得できないためlist_videosを使って取得
  #
  def view_count(item)
    @view_count ||= @youtube.list_videos(
                      :statistics,
                      id: item.id.video_id,
                      fields: 'items(statistics(view_count))'
                    ).items[0].statistics.view_count
  end

  #
  # Youtube動画情報を収集
  #
  def youtube_videos
    @youtube_videos ||= fetch_youtube_videos.items.map do |item|
      {
        video_id: item.id.video_id,
        title: item.snippet.title,
        thumbnail: item.snippet.thumbnails.medium.url,
        view_count: view_count(item),
        published_at: item.snippet.published_at
      }
    end
  end

  #
  # 動画情報を保存
  #
  def save_videos
    youtube_videos.each do |result|
      spot.videos.create!(
        video_id: result[:video_id],
        title: result[:title],
        thumbnail: result[:thumbnail],
        view_count: result[:view_count],
        published_at: result[:published_at]
      )
    end
  end

  #
  # 動画情報を更新
  #
  def update_videos
    videos.each_with_index do |result, index|
      result.update!(
        video_id: youtube_videos[index][:video_id],
        title: youtube_videos[index][:title],
        thumbnail: youtube_videos[index][:thumbnail],
        view_count: youtube_videos[index][:view_count],
        updated_at: Time.current
      )
    end
  end

  def videos
    @videos ||= spot.videos
  end

  def spot
    @spot ||= Spot.find(params[:spot_id])
  end
end

修正ポイント

  1. シリアライザの活用
    active_model_serializers を導入したことでレスポンスの形式を簡易なコードですっきり整形
  2. キャッシュの活用
    何度も呼ばれる関数は||=を使うことで何度もDBアクセスが走らないようにして時短な処理に修正
  3. モジュールの変更
    モデルのフォルダ内のにあったモジュールから、コントローラーのフォルダ内に移動
  4. コントローラーのスリム化
    DBへの保存・更新処理を実行するかの条件分岐はコントローラーに持たせ、保存・更新処理はモジュールで全て完結するよう修正

修正結果

設計と実装の中身を中味を改修したことで、操作性はそのままに動画一覧表示の処理速度が1/2にまで短縮されました✨

  • 修正前
    修正前.png

  • 修正後
    修正後.png

おわりに

今回の出来事を通じて、昔の自分のコードをレビューして振り返ることは、今の自分がどれほど成長したのか実感できるいい機会になるなと感じました。また、仕様は変えず設計や実装処理の内容を修正することでパフォーマンスを向上させる作業は、実務でも大奥はないと思うので、自分の力を試すことができるなとも思いました。
眠ったままの個人サービスをお持ちの方は、ぜひ過去の自分のコードを振り返り、今の自分のレベルを体感してみてください。

ここまで読んで頂いた皆様、ありがとうございました！

11
7
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
11
7