refactor: YAGNI/DRY に沿って news.rake のコードを共通化 #1760
Merged
+72
−118
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 不要なヘルパー関数 3 つを削除（safe_open, fetch_rss_items, item_to_hash） - 過剰なファイル存在チェックを削除（YAGNI） - 不要な require 文を削除（net/http, uri, yaml, time, active_support/broadcast_logger） - 変数名をより明確に（yaml_path → news_yaml_path, file_logger → logger_file） - RSS フィード処理をインライン化で直接実装 - コード行数を 50% 削減（171行 → 135行） 全テスト通過を確認済み。機能に変更なし。
- TEST_NEWS_FEED 定数を追加し、明示的な命名に - RSS_FEED_LIST の定義をより読みやすく
- permitted_classes: [Time] を削除（実際のデータに Time オブジェクトなし） - aliases: true を削除（実際のデータに YAML エイリアスなし） - YAGNI 原則により実際に必要のないオプションを削除 - 全181テストが正常に通過することを確認
- raw 変数を削除して直接 entries に代入 - ['news'] || [] の不要な処理を削除 - YAMLファイルが配列を直接返すため || [] も不要 - 2行削減でより直接的な実装に
yasulab
force-pushed
the
refactor-news-rake-task
branch
from
41fc393 to
4647c2b
Compare
yasulab
changed the title
- if/elsif構文に統一して条件分岐を簡素化 - 変数名を明確化（all_items → merged_items） - ソート処理をメソッドチェーンで一行に統合 - コメントを具体的な処理内容に改善 - YAGNI原則に基づく不要な複雑さの除去
- それぞれのパスを定数化（NEWS_YAML_PATH, NEWS_LOG_PATH） - fetch と upsert 両タスクで同じ定数を使用 (less hard codes) - Single Source of Truth の原則を徹底
- entries → news_items: より意図が明確な変数名 - attrs → item: 簡潔で分かりやすい命名 - new_count → created_count: fetch タスクとの一貫性 - is_new → is_new_record: より具体的で明確 - 三項演算子を if/unless に変更して可読性向上 - 両タスク間での用語統一でコード全体の一貫性確保
yasulab
changed the title
- each_with_index → each.with_index(1) で開始インデックスを明示 - existing_max_id + index + 1 → existing_max_id + index で計算簡素化 - インデックスの起点が「1」である点が、より明確になるコードに変更
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
概要
lib/tasks/news.rakeの可読性と保守性を向上させるYAGNI/DRY原則に基づく包括的リファクタリング🎯 主要改善
1. 条件分岐の簡素化（YAGNI原則）
if/elsifに統一existing_item.nil?での明確な新規判定2. 定数による設定集約（DRY原則）
NEWS_YAML_PATH: YAMLファイルパスを一元管理NEWS_LOG_PATH: ログファイルパスを一元管理3. タスク間の変数名統一
fetchタスク:created_items,updated_itemsupsertタスク:created_count,updated_countentries→news_items: より意図が明確attrs→item: 簡潔で統一感のある命名4. コード品質の向上
sorted_items等）を削除itパラメータを活用📊 改善効果
if/elsif✅ 検証済み
bin/rails news:fetchの結果に変更なしdb/news.ymlが正しく保持されることを確認🔧 技術的詳細
適用原則
変更サマリー
lib/tasks/news.rake📈 長期的メリット
🚀 実装アプローチ
段階的リファクタリングにより安全性を確保：
各段階でテストを実行し、既存動作の維持を確認しました。