ChatGPTに昔つくったコードのリファクタリングをしてもらおうと思い、まずは改善点を出してもらったら
— からあげ (@karaage0703) August 26, 2023
「あ、ああああああぁーーーー！」って感じになってます。 pic.twitter.com/zOVoAJdSfI
Data Analysis: Network X
Code
# -*- coding: utf-8 -*- """ Created on Tue Aug 22 16:20:50 2023 @author: takashige.aoki """ import pandas as pd import networkx as nx import matplotlib.pyplot as plt # データの読み込み (CSV形式を仮定) df = pd.read_csv('xxx.csv') # 適切なファイル名を設定してください df = df.query('uu>1000') # グラフの作成 G = nx.Graph() # エッジの追加 for _, row in df.iterrows(): G.add_edge(row['muskcol1'], row['muskcol2'], weight=row['uu']) plt.figure(figsize=(15,15)) pos = nx.spring_layout(G, k=5) # レイアウトの指定 pos = nx.circular_layout(G) edges = G.edges(data=True) edge_widths = [d['weight'] / 1000 for _, _, d in edges] # エッジの描画 (透明度を0.3に指定) nx.draw_networkx_edges(G, pos, width=edge_widths, edge_color='black', alpha=0.3) # ノードの描画 (透明度はデフォルトの1.0) nx.draw_networkx_nodes(G, pos, node_size=2000, node_color='skyblue', alpha=0.8) # ラベルの描画 labels = nx.draw_networkx_labels(G, pos, font_color='black') plt.title('xxxRelationships') plt.savefig("networkx_xxx.png", dpi=300) plt.show() #%% xxx data # データの読み込み (CSV形式を仮定) df = pd.read_csv('xxx.csv') # 適切なファイル名を設定してください df = df.query('uu>10000') # グラフの作成 G = nx.Graph() # エッジの追加 for _, row in df.iterrows(): G.add_edge(row['muskcol1'], row['muskcol2'], weight=row['uu']) plt.figure(figsize=(15,15)) pos = nx.spring_layout(G, k=5) # レイアウトの指定 pos = nx.circular_layout(G) edges = G.edges(data=True) edge_widths = [d['weight'] / 10000 for _, _, d in edges] # エッジの描画 (透明度を0.3に指定) nx.draw_networkx_edges(G, pos, width=edge_widths, edge_color='black', alpha=0.3) # ノードの描画 (透明度はデフォルトの1.0) nx.draw_networkx_nodes(G, pos, node_size=2000, node_color='skyblue', alpha=0.8) # ラベルの描画 labels = nx.draw_networkx_labels(G, pos, font_color='black') plt.title('xxx Relationships') plt.savefig("networkx_xxx.png", dpi=300) plt.show()
Refactoring
このプログラムは、`networkx`ライブラリを用いてCSVファイルから読み込んだデータを基にxxx間の関係を表すグラフを描画するものです。以下にリファクタリングの点、課題や懸念、リスクを列挙します。
### リファクタリングの点
1. **コードの重複**: グラフを描画する部分が2回繰り返されている。関数にまとめて一度だけ書くことで、コードの再利用性と可読性を向上させる。
2. **ハードコードされた数値**: `query('uu>1000')`や`query('uu>10000')`のようにハードコードされた数値が存在する。これを変数または引数で指定できるようにする。
3. **ファイル名のハードコード**: CSVファイル名がハードコードされている。引数や設定ファイルで指定できるようにする。
4. **レイアウトの選択**: `nx.spring_layout`と`nx.circular_layout`が両方とも指定されているが、後者が上書きする形になっている。
### 課題や懸念
1. **エラーハンドリング**: CSVファイルが存在しない、または期待する形式でない場合のエラーハンドリングが不足している。
2. **リソースの解放**: プロットの保存や表示が完了した後で、不要なリソースが解放されていない。
3. **変数名の明瞭性**: `df`, `G`など、変数名が短すぎて何を示しているのかが一目でわかりにくい。
4. **コメントの不足**: 何をするためのコードなのか、各部分で何が行われているのかを説明するコメントが少ない。
### リスク
1. **メモリ消費**: 大規模なCSVファイルを読み込むと、メモリが不足する可能性がある。
2. **ファイルの存在**: 指定されたCSVファイルが存在しない場合、または不正な形式の場合にプログラムがクラッシュする可能性がある。
3. **データの質**: `'uu'`の値が非常に大きいか小さい場合、グラフが読みにくくなるかもしれません。
4. **処理速度**: 大規模なデータセットの場合、グラフの作成に時間がかかる可能性があります。
これらの点を考慮してリファクタリングを行うと、より安全で可読性とメンテナビリティの高いコードになるでしょう。
Data Collecting: Scraping
Code
# -*- coding: utf-8 -*- """ Created on Tue Aug 1 10:27:51 2023 @author: takashige.aoki """ #%% # import modules import re import csv import time import datetime import pandas as pd from bs4 import BeautifulSoup from selenium import webdriver from selenium.webdriver.chrome.options import Options from selenium.webdriver.chrome.service import Service from webdriver_manager.chrome import ChromeDriverManager #%% # driver starting # Start the WebDriver and navigate to the URL # chrome_options = Options() # driver = webdriver.Chrome( # executable_path=r'C:\Users\takashige.aoki\AppData\Local\SeleniumBasic\chromedriver.exe', # options=chrome_options) # service = Service(executable_path=r'C:\Users\takashige.aoki\AppData\Local\SeleniumBasic\chromedriver.exe') # driver = webdriver.Chrome(service=service) driver = webdriver.Chrome(service=Service(ChromeDriverManager().install())) # 最新バージョンのChromeDriverをダウンロード # driver = webdriver.Chrome(executable_path=ChromeDriverManager().install()) #%% # access amazon def scrape_amazon(search_word): url = f'https://www.amazon.co.jp/s?k={search_word}' # Start the WebDriver and navigate to the URL driver.get(url) time.sleep(2) # Give some time for the page to load # Extract the HTML content and create a BeautifulSoup object page_source = driver.page_source soup = BeautifulSoup(page_source, 'html.parser') # 任意の文字列で始まるclass属性を持つdiv要素を見つける target_class_prefix = 's-widget-container s-spacing-small s-widget-container-height-small celwidget slot=MAIN template=SEARCH_RESULTS widgetId=search-results' target_divs = soup.find_all('div', class_=lambda c: c and c.startswith(target_class_prefix)) #%% # 結果を格納するリスト links = [] names = [] prices = [] ads = [] recommended = [] if len(target_divs) == 0: print("no result") data = {'search_word':search_word, 'Title': names, 'Link': links, 'price': prices, 'ad': ads, 'recommended': recommended} df = pd.DataFrame(data) return df #%% ad_text = 'この広告は、検索クエリに対する商品の関連性に基づいて表示されています。' recommend_text = '「おすすめ出品」' # div要素の中のspan要素とリンクを抽出 for div in target_divs: name = div.find('span', class_=re.compile(r'a-text-normal')).text if div.find('span', class_=re.compile(r'a-text-normal')) else "" price_span = div.find('span', class_=re.compile(r'a-price-whole')) price = remove_comma_and_convert_to_numeric(price_span.text) if price_span else None ad = ad_text in div.text recommend = recommend_text in div.text link = 'https://www.amazon.co.jp' + div.find('a')['href'] print(ad, recommend) #%% if recommend: print("recommend") price = None driver.get(link) time.sleep(2) # Extract the HTML content and create a BeautifulSoup object for the new page page_source = driver.page_source soup = BeautifulSoup(page_source, 'html.parser') # BeautifulSoupオブジェクトからタイトルが「すべての出品を見る」となっている<a>タグを探す a_tag = soup.find('a', string=re.compile('すべての出品を見る')) # <a>タグが存在する場合 if a_tag: link_url = a_tag['href'] # 取得したリンクに移動 driver.get('https://www.amazon.co.jp' + link_url) time.sleep(0.5) #Reload HTML page_source = driver.page_source soup = BeautifulSoup(page_source, 'html.parser') target_class_prefix = 'a-section a-spacing-none a-padding-base aod-information-block aod-clear-float' target_divs = soup.find_all('div', class_=lambda c: c and c.startswith(target_class_prefix)) #%% for div in target_divs: price_span = div.find('span', class_=re.compile(r'a-price-whole')) price = remove_comma_and_convert_to_numeric(price_span.text) if price_span else None links.append(link) names.append(name) prices.append(price) ads.append(ad) recommended.append(recommend) data = {'search_word':search_word, 'Title': names, 'Link': links, 'price': prices, 'ad': ads, 'recommended': recommended} df = pd.DataFrame(data) return df #%% links.append(link) names.append(name) prices.append(price) ads.append(ad) recommended.append(recommend) data = {'search_word':search_word, 'Title': names, 'Link': links, 'price': prices, 'ad': ads, 'recommended': recommended} df = pd.DataFrame(data) return df def remove_comma_and_convert_to_numeric(value): try: return pd.to_numeric(value.replace(',', '')) except ValueError: return None def main(): all_results_df = pd.DataFrame() # Replace 'path/to/your/csv_file.csv' with the actual path to your CSV file containing JAN codes with open('search_word.csv', newline='') as csvfile: reader = csv.reader(csvfile) for row in reader: search_word = row[0] print(search_word) search_result_df = scrape_amazon(search_word) all_results_df = pd.concat([all_results_df, search_result_df]) driver.quit() #%% current_date = datetime.datetime.now().strftime('%Y%m%d') file_name = f'result_detail_{current_date}.csv' all_results_df.to_csv(file_name) # Excel形式で保存 file_name = f'result_summary_{current_date}.xlsx' all_results_df.to_excel(file_name) #%% summary_df = pd.DataFrame() # IDごとの最小値を計算して新しい列'Min_Value'として追加 all_results_df['Min_Value'] = all_results_df.groupby('search_word')['price'].transform('min') # 最小値の行だけを抽出 summary_df = all_results_df[all_results_df['price'] == all_results_df['Min_Value']] # 'Min_Value'列を削除 summary_df = summary_df.drop(columns=['price']) file_name = f'result_summary_{current_date}.csv' summary_df.to_csv(file_name) # Excel形式で保存 file_name = f'result_summary_{current_date}.xlsx' summary_df.to_excel(file_name) print('done!') print(all_results_df.info()) print(all_results_df.head()) if __name__ == "__main__": main()
Refactoring
以下はリファクタリング、課題、懸念、およびリスクの一覧です。
### リファクタリング
1. **コードの分割**: `scrape_amazon()` は非常に長く、多くの責任を持っています。この関数を小さな単位に分割すると、テストとデバッグが容易になります。
2. **グローバル変数**: ドライバー（`driver`）がグローバル変数として設定されています。これを関数の引数またはクラスの属性として渡すことを検討してください。
3. **Magic Numbers/Strings**: 例えば、時間（`time.sleep(2)`）や特定のクラス名はコード内でハードコードされています。これらを定数として設定すると、変更が容易になります。
4. **ファイル操作**: ファイルの読み書きが複数の場所で行われています。これを一箇所にまとめると、変更が容易になります。
5. **エラーハンドリング**: 例外処理が不十分です。特に、Webスクレイピングには多くのエラーが発生する可能性があります。
6. **コードの重複**: いくつかの部分でコードが重複しています（例：CSVとExcelでの保存）。### 課題と懸念
1. **パフォーマンス**: `time.sleep()` が頻繁に使われているため、プログラムのパフォーマンスに影響を与える可能性があります。
2. **メンテナンス**: HTML構造が変更されると、すぐにコードが壊れる可能性があります。
3. **可読性**: 長い関数、ネストが深い、コメントが少ない等、可読性に問題があります。
4. **ローカルパス**: コメントアウトされた部分でローカルパスが指定されています。これは他の環境での実行に問題を引き起こす可能性があります。### リスク
1. **法的リスク**: Webスクレイピングは法的に問題を引き起こす可能性があります。
2. **データの品質**: 推奨される商品や広告商品など、不必要なデータが抽出される可能性があります。
3. **安定性**: インターネットの接続問題、Amazonのページ構造の変更、またはSeleniumやBeautifulSoupの将来のバージョンの変更などにより、コードが壊れる可能性があります。このコードを効率的かつ安全にするためには、上記のポイントに対処する必要があります。
Reference
【初学者向け】Python社内DX7選 - Qiita
[Python]NetworkXでQiitaのタグ関係図を描く - Qiita
お客様が、Amazonをスクレイピングするのは禁止です。
このことは、利用規約に明示的に記載されています。でも、お客様でないなら、スクレイピングをしてもいいのでは？
もっと言うと、利用規約に同意していないなら、スクレイピングをしてもいいのでは？みなさん、このように思いませんか？
私は思います。
