質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

新規登録して質問してみよう
ただいま回答率
85.50%
Python 3.x

Python 3はPythonプログラミング言語の最新バージョンであり、2008年12月3日にリリースされました。

Q&A

解決済

4回答

957閲覧

PYTHONソースファイルのリファクタリング

yuujiMotoki

総合スコア90

Python 3.x

Python 3はPythonプログラミング言語の最新バージョンであり、2008年12月3日にリリースされました。

0グッド

1クリップ

投稿2020/02/14 00:44

編集2020/02/14 02:05

仕事でPYTHON-FLASKを使ったアプリケーションを作っています。

ほとんどの処理をapp.pyというファイルに入れています。
理由としては、PYTHON自体がIMPORT等を使っても、動作としては
全部読み込んで動くスクリプトなので、特に分割するメリットを感じなかったためです。

ところが、直近になって私本人の移動の話があり、
この理解しにくい、ソースコードを別担当者に引き継ぐ必要が出てきました。

まずはコード自体が長すぎるために、これを短く書く検討を始めています。

そこで添付のようなコードの場合は、どこを、どのように切れば
このようなコード縮小が可能でしょうか?

いまだ未熟者のため、リファクタリングで動かなくなるリスクを懸念して
手が付けられていない状況です。

参考のソースファイルを下記に示します。

実際には、これらの5倍ぐらいのコードがあります。
忌憚のないご意見を頂けましたら幸いです。

python

1#app.py 2 3import os 4import sys 5import io 6import time 7import numpy as np 8import pandas as pd 9import cv2 10from flask import Flask, render_template, request, redirect, url_for, send_from_directory, session, flash, make_response 11import pdb 12import PyPDF2 13import random 14import json 15from flask_sqlalchemy import SQLAlchemy 16from sqlalchemy import create_engine 17from sqlalchemy.orm import sessionmaker 18import sqlite3 19from datetime import datetime 20from werkzeug.utils import secure_filename 21import Fxx_COUNT as fsc 22import secrets 23import logging 24import string 25 26 27LOGFILE = "LOGFILE.log" 28 29app = Flask(__name__) 30 31app.secret_key = 'any random string' 32app.logger.setLevel(logging.DEBUG) 33fh = logging.FileHandler(LOGFILE) 34fh.setLevel(logging.DEBUG) 35app.logger.addHandler(fh) 36 37UPLOAD_FOLDER = './uploads/' 38STRAGE_FOLDER = './strages/' 39TEMP_FOLDER = './temp/' 40STATIC_FOLDER = './static/' 41 42ALLOWED_EXTENSIONS = set(['png', 'jpg', 'PNG', 'JPG', 'pdf']) 43IMAGE_WIDTH = 640 44app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///members.db' 45app.config['UPLOAD_FOLDER'] = UPLOAD_FOLDER # MCOのオリジナルをアップロードするフォルダ 46app.config['STRAGE_FOLDER'] = STRAGE_FOLDER # 処理済ファイルをアップロードするフォルダ 47app.config['TEMP_FOLDER'] = TEMP_FOLDER # 処理済ファイルをアップロードするフォルダ 48app.config['STATIC_FOLDER'] = STATIC_FOLDER # 処理済ファイルをアップロードするフォルダ 49app.config['SECRET_KEY'] = os.urandom(24) 50PATTERN_FILE = "Fxx.png" 51 52#db = SQLAlchemy() 53# db.init_app(app) 54 55 56def allowed_file(filename): 57 return '.' in filename and \ 58 filename.rsplit('.', 1)[1] in ALLOWED_EXTENSIONS 59 60# members.dbから、CUSTOMERSを読み込んで、VARへ挿入する 61@app.route('/customer') 62def customer(): 63 conn = sqlite3.connect(app.config['DATABASE']) 64 cur = conn.cursor() 65 cur.execute("SELECT * FROM customers") 66 var = cur.fetchall() 67 conn.close() 68 return render_template('customer.html', var=var) 69 70# items sign up 71@app.route('/customer_signed_up', methods=['GET', 'POST']) 72def customer_signed_up(): 73 # pdb.set_trace() 74 session['username'] = request.form['username'] 75 password = request.form['password'] 76 conn = sqlite3.connect(app.config['DATABASE']) 77 cur = conn.cursor() 78 cur.execute("INSERT INTO customers(username,password) VALUES (?,?)", 79 (session['username'], password)) 80 conn.commit() 81 conn.close() 82 return redirect(url_for('logout')) 83 84# items delete 85@app.route('/customer_delete', methods=['GET', 'POST']) 86def customer_delete(): 87 session['username'] = request.form['username'] 88 password = request.form['password'] 89 conn = sqlite3.connect(app.config['DATABASE']) 90 cur = conn.cursor() 91 cur.execute("delete from customers where username=?", (session['username'],)) 92 cur.execute("delete from response where username=?", (session['username'],)) 93 cur.execute("delete from response2 where username=?", (session['username'],)) 94 conn.commit() 95 conn.close() 96 return redirect(url_for('logout')) 97 98# ログイン処理 ← SESSION変数にログインしたユーザーの名前が入る 99@app.route('/customer_logged_in', methods=['GET', 'POST']) 100def customer_logged_in(): 101 session['username'] = request.form['username'] 102 dir1=os.path.join(app.config['UPLOAD_FOLDER'],session['username']) 103 if not os.path.exists(dir1): 104 os.mkdir(dir1) 105 106 app.logger.info('%s logged in successfully', session['username']) 107 return redirect(url_for('homepage_customer')) 108 109# メインページへの誘導のため、データベースのテーブルを呼び出す 110@app.route('/') 111@app.route('/homepage_customer') 112def homepage_customer(): 113 if 'username' in session: 114 conn = sqlite3.connect(app.config['DATABASE']) 115 cur = conn.cursor() 116 cur.execute("SELECT * FROM response WHERE username=? order by url asc" , 117 (session['username'],)) 118 response = cur.fetchall() 119 cur.execute("SELECT * FROM response2 WHERE username=?", 120 (session['username'],)) 121 response2 = cur.fetchall() 122 conn.close() 123 124 image = {} 125 my_images = [] # イメージ画像の辞書 126 127 for id, name, file1, number, sender in response: 128 if file1.endswith('.png'): 129 image['url'] = file1 130 image['id']=id 131 image['message']=number 132 if sender == 0: 133 sender = 0 134 elif not sender: 135 sender = -1 136 image['sender']=sender 137 my_images.append(image.copy()) 138 139 pimage = {} 140 p_images = [] 141 142 for id,name, file2, sel ,dum,dum in response2: 143 if file2.endswith('.png'): 144 pimage['id'] = id 145 pimage['url'] = file2 146 p_images.append(pimage.copy()) 147 148 return render_template('index.html', items=my_images, p_items=p_images, USER=session['username']) 149 return redirect('customer') 150 151@app.route('/logout') 152def logout(): 153 # remove the username from the session if its there 154 session.pop('username', None) 155 return redirect('/') 156 157# サーバーへPDFをアップロードする 158@app.route('/send', methods=['GET', 'POST']) 159def send(): 160 if request.method == 'POST': 161 img_file = request.files['img_file'] 162 if img_file and img_file.filename.endswith('.pdf'): 163 #pdb.set_trace() 164 saveFileName = path_( app.config['UPLOAD_FOLDER'],session['username'], secure_filename(img_file.filename)) 165 if os.path.isfile(saveFileName): 166 flash('ファイル名が重複しています。重複しないページのみを更新します。') 167 else: 168 img_file.save(saveFileName) 169 else: 170 flash('pdfファイルではありません') 171 saveFileName = "" 172 return render_template('index.html', upload_img_url=saveFileName) 173 else: 174 return redirect('/') 175 176def path_(a,b,c): 177 d= os.path.join(a,b) 178 return os.path.join(d,c) 179 180# サーバーへPattern をアップロードする 181@app.route('/pattern', methods=['GET', 'POST']) 182def pattern(): 183 if request.method == 'POST': 184 img_file = request.files['img_file'] 185 if img_file and img_file.filename.endswith('.png'): 186 saveFileName = os.path.join( 187 app.config['UPLOAD_FOLDER'], datetime.now().strftime("%Y%m%d_%H%M%S_")+secure_filename(img_file.filename)) 188 if os.path.isfile(saveFileName): 189 flash('ファイル名が重複しています') 190 saveFileName = "" 191 else: 192 img_file.save(saveFileName) 193 conn = sqlite3.connect(app.config['DATABASE']) 194 cur = conn.cursor() 195 cur.execute("INSERT INTO response2(username,url,choose,circle,line) VALUES (?,?,?,?,?)", 196 (session['username'], saveFileName, 1 , 10 , 6 )) 197 conn.commit() 198 conn.close() 199 else: 200 flash('pngファイルではありません') 201 saveFileName = "" 202 return redirect('/') 203 else: 204 return redirect('/') 205 206 207 208中略 209 210##python app nonx ---> Format for Non-X without Fxx/Sxx 211##python app ---> Format for X with Fxx/Sxx 212 213if __name__ == '__main__': 214 app.config['USE_FxxSxx_NOTATION'] = True 215 app.config['PORT']='8080' 216 app.config['DATABASE']='members.db' 217 app.config['ADMIN_PASS']="xxxx" ## X-USER 218 219 if len(sys.argv)>=2: 220 if sys.argv[1]=="nonx": 221 app.config['USE_FxxSxx_NOTATION'] = False 222 app.config['PORT'] = '8888' 223 app.config['DATABASE']='members2.db' 224 app.config['ADMIN_PASS']="xxxx" ## NONX-USER 225 app.run('localhost', port=app.config['PORT'] ,debug=True)

気になる質問をクリップする

クリップした質問は、後からいつでもMYページで確認できます。

またクリップした質問に回答があった際、通知やメールを受け取ることができます。

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

dodox86

2020/02/14 01:12

「引き継ぐ」と言うのは、質問文中の(yuujiMotokiさんが書かれたと思しき)ご提示のコードを、別の人に渡す、と言うことでしょうか。
siruku6

2020/02/14 02:50 編集

リファクタリングで動作が変わる恐れがあるということであれば、自分でリファクタリングするよりは、unittestだけ実装してあげてリファクタリングは引き継いだ相手にしてもらった方が安全でかつ、将来にわたってプラスになるような気がしました。 実際にリファクタリングで動かなくなるリスクも大幅に減らせます。 ただし、周囲の理解が得られるかはわかりません、、、 スマホしか現在使えず、ソース全体を見渡すのが難しいのでひとまずこれだけお伝えしてみます。
guest

回答4

0

ご提示のコードは225行ありました。
この5倍だとしても1125行でそれほど多いとは思えません。
リファクタリングして余計わからなくなる可能性の方が高いです。(分割すると増えますし)

それよりも、移動して引き継ぐなら引き継ぎ資料を作った方が良いです。
もしも設計書がないならそれも作りましょう。
私が引き継ぐ場合、設計書の名前はなんでもいいですが、以下を記載してほしいです。
設計書は自分のためにもなりますのであって損はありません。

・そのアプリが何をするのかの概要
・そのアプリにある機能一覧
・ファイル一覧
何というファイルがあるというもの
例えば、LOGFILE.logはどこにできるとか。
・機能毎の関連ファイル(関数)一覧
機能が使っているファイルの一覧
ファイル一覧とかぶるが、あると便利
・機能毎のフロー
何にアクセスするとどの関数が呼ばれるなど
・機能毎のInput/Output
何を受け取って、何を出力するかなど
受け取る方法(GetとかPOSTとか、どこかのフォルダの●○.txtを読むとか)
出力する方法(●にoutput.txtができますとか、DBのxxtableに1行追加(更新)されますとか)
・使っているpythonバージョン
・使っているflaskバージョン
・使っているmodule一覧、およびそれを使っている機能
・使っているDBの名前
・DBのCreate文
・最初の環境を作るのに必要なInsert文
できれば、本番環境の最初とテスト環境の最初にできるinsertがあるとうれしい。
・logファイルのローテートの設定など
・sqliteっぽいですが、ファイルが無い場合どうするのか
・このアプリを動かすOS
・OSの設定
・使っているWebサーバの名前(Apacheとかnginxとか)と設定(configの中身と意味)
・wsgiではなくgunicornとか使っているならその設定と、接続方法
Apacheを起動するとアプリも起動しますとか、アプリは別に動いているので起動が必要ですとか
簡単に言うとWebアプリを動かすためのサーバ構築手順書ですね
メモリ量とか、必要なアクセス権(/var/www/hogehoge/に777が必要ですみたいな)
・E-R図

他にも
・(会社の他の人からいままで受けた)要望
・要望の対応状況と未対応ならその理由
・(会社の別の人が使っているなら)更新するための手順
全社に連絡して、18時以降じゃないと更新できませんとかあるかも?
・サーバの管理者とかサーバ名とかもあれば
必要ならIDとパスワードも

投稿2020/02/14 04:09

FiroProchainezo

総合スコア2387

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

dodox86

2020/02/14 05:13 編集

保守する側の立場からも、「リファクタリング」の観点からも、高評価させていただきました。 > 添付のようなコードの場合は、どこを、どのように切ればこのようなコード縮小が可能でしょうか? と言うのがご質問の本筋なので回答はしませんでしたが、本回答のFiroProchainezoさんと、コメントでのsiruku6さんに同意します。 引継ぎを受け、保守を任される立場からすると現行の動いているコードと、(リファクタリングしたと称する)新しいコードがあり、動きが違った場合、どちらが正しいのか分かりません。それを担保するのがunittest(単体テスト)コードのはずですが、それができない場合はドキュメントで「何をしているのか(求めているのか)」を明示しておかないと、保守ができません。「リファクタリング」とは一般的に外側の挙動はまったく同じに中身を改良することを指しますが、単体テストが無い場合は保証できないと思います。
guest

0

FiroProchainezoさんの回答ともかぶりますが、1000行程度であれば、ファイル分割はしない方がたぶん親切です。分割しない設計で作ってしまったものをばらして訳わからなくなるよりは良いでしょう。必要なら引き継いだ人がやります。

たぶん引き継ぎ資料を頑張って作ったほうが正しいのでコードに関しては最小限のことだけをする方針にして、PEP8(公式コーディング規約)に準拠させて、あまり使ってない便利関数とかは消して(path_とか要らないと思うんですが)可能ならロジックを整理して(本当にすぐできることだけです。if文が多いので何箇所かは整理できるでしょう。たとえばhomepage_customernot inで判定した方がきれいじゃん、とか)、あとはdocstringとかコメントとか型ヒントとかを適宜入れておくという感じでいいのでは。一日かからないと思います。

投稿2020/02/14 05:24

編集2020/02/14 05:31
hayataka2049

総合スコア30933

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

0

自己解決

まだ引継ぎができていませんが、ソースファイルのまま
引き渡すことにいたしました。

逆に、へたにオブジェクト思考かしていないので、
可読性はこのままにしようと思いました。

投稿2020/04/24 12:39

yuujiMotoki

総合スコア90

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

Zuishin

2020/04/24 23:02

最低の自己解決。 長文のコードを読ませておいて長期にわたって放置した挙げ句、ついた回答に何も触れず、質問自体をひっくり返している。引き継ぎ資料や公式ドキュメントをどのようにしたかの報告もない。 つまり、報告もなく、連絡もせず、相談はひっくり返している。
guest

0

公式ドキュメントを参考にしてみては?
より大きなアプリケーション — Flask v0.5.1 documentation
https://a2c.bitbucket.io/flask/patterns/packages.html

他には「flask ファイル構成」とかでググるといろいろ見つかります。

投稿2020/02/14 03:47

shiracamus

総合スコア5406

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

15分調べてもわからないことは
teratailで質問しよう!

ただいまの回答率
85.50%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問