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

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

ただいまの
回答率

87.49%

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

解決済

回答 4

投稿 編集

  • 評価
  • クリップ 1
  • VIEW 783

score 59

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

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

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

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

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

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

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

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

#app.py

import os
import sys
import io
import time
import numpy as np
import pandas as pd
import cv2
from flask import Flask, render_template, request, redirect, url_for, send_from_directory, session, flash, make_response
import pdb
import PyPDF2
import random
import json
from flask_sqlalchemy import SQLAlchemy
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
import sqlite3
from datetime import datetime
from werkzeug.utils import secure_filename
import Fxx_COUNT as fsc
import secrets
import logging
import string


LOGFILE = "LOGFILE.log"

app = Flask(__name__)

app.secret_key = 'any random string'
app.logger.setLevel(logging.DEBUG)
fh = logging.FileHandler(LOGFILE)
fh.setLevel(logging.DEBUG)
app.logger.addHandler(fh)

UPLOAD_FOLDER = './uploads/'
STRAGE_FOLDER = './strages/'
TEMP_FOLDER = './temp/'
STATIC_FOLDER = './static/'

ALLOWED_EXTENSIONS = set(['png', 'jpg', 'PNG', 'JPG', 'pdf'])
IMAGE_WIDTH = 640
app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///members.db'
app.config['UPLOAD_FOLDER'] = UPLOAD_FOLDER  # MCOのオリジナルをアップロードするフォルダ
app.config['STRAGE_FOLDER'] = STRAGE_FOLDER  # 処理済ファイルをアップロードするフォルダ
app.config['TEMP_FOLDER'] = TEMP_FOLDER  # 処理済ファイルをアップロードするフォルダ
app.config['STATIC_FOLDER'] = STATIC_FOLDER  # 処理済ファイルをアップロードするフォルダ
app.config['SECRET_KEY'] = os.urandom(24)
PATTERN_FILE = "Fxx.png"

#db = SQLAlchemy()
# db.init_app(app)


def allowed_file(filename):
    return '.' in filename and \
        filename.rsplit('.', 1)[1] in ALLOWED_EXTENSIONS

# members.dbから、CUSTOMERSを読み込んで、VARへ挿入する
@app.route('/customer')
def customer():
    conn = sqlite3.connect(app.config['DATABASE'])
    cur = conn.cursor()
    cur.execute("SELECT * FROM customers")
    var = cur.fetchall()
    conn.close()
    return render_template('customer.html', var=var)

# items sign up
@app.route('/customer_signed_up', methods=['GET', 'POST'])
def customer_signed_up():
    # pdb.set_trace()
    session['username'] = request.form['username']
    password = request.form['password']
    conn = sqlite3.connect(app.config['DATABASE'])
    cur = conn.cursor()
    cur.execute("INSERT INTO customers(username,password) VALUES (?,?)",
                (session['username'], password))
    conn.commit()
    conn.close()
    return redirect(url_for('logout'))

# items delete
@app.route('/customer_delete', methods=['GET', 'POST'])
def customer_delete():
    session['username'] = request.form['username']
    password = request.form['password']
    conn = sqlite3.connect(app.config['DATABASE'])
    cur = conn.cursor()
    cur.execute("delete from customers where username=?", (session['username'],))
    cur.execute("delete from response where username=?", (session['username'],))
    cur.execute("delete from response2 where username=?", (session['username'],))
    conn.commit()
    conn.close()
    return redirect(url_for('logout'))

# ログイン処理 ← SESSION変数にログインしたユーザーの名前が入る
@app.route('/customer_logged_in', methods=['GET', 'POST'])
def customer_logged_in():
    session['username'] = request.form['username']
    dir1=os.path.join(app.config['UPLOAD_FOLDER'],session['username'])
    if not os.path.exists(dir1):
        os.mkdir(dir1)

    app.logger.info('%s logged in successfully', session['username'])
    return redirect(url_for('homepage_customer'))

# メインページへの誘導のため、データベースのテーブルを呼び出す
@app.route('/')
@app.route('/homepage_customer')
def homepage_customer():
    if 'username' in session:
        conn = sqlite3.connect(app.config['DATABASE'])
        cur = conn.cursor()
        cur.execute("SELECT * FROM response WHERE username=? order by url asc" ,
                    (session['username'],))
        response = cur.fetchall()
        cur.execute("SELECT * FROM response2 WHERE username=?",
                    (session['username'],))
        response2 = cur.fetchall()
        conn.close()

        image = {}
        my_images = []  # イメージ画像の辞書

        for id, name, file1, number, sender in response:
            if file1.endswith('.png'):
                image['url'] = file1
                image['id']=id
                image['message']=number
                if sender == 0:
                    sender = 0  
                elif not sender:
                    sender = -1
                image['sender']=sender
                my_images.append(image.copy())

        pimage = {}
        p_images = []

        for id,name, file2, sel ,dum,dum in response2:
            if file2.endswith('.png'):
                pimage['id'] = id
                pimage['url'] = file2
                p_images.append(pimage.copy())

        return render_template('index.html', items=my_images, p_items=p_images, USER=session['username'])
    return redirect('customer')

@app.route('/logout')
def logout():
    # remove the username from the session if its there
    session.pop('username', None)
    return redirect('/')

# サーバーへPDFをアップロードする
@app.route('/send', methods=['GET', 'POST'])
def send():
    if request.method == 'POST':
        img_file = request.files['img_file']
        if img_file and img_file.filename.endswith('.pdf'):
            #pdb.set_trace()
            saveFileName = path_( app.config['UPLOAD_FOLDER'],session['username'], secure_filename(img_file.filename))     
            if os.path.isfile(saveFileName):
                flash('ファイル名が重複しています。重複しないページのみを更新します。')
            else:
                img_file.save(saveFileName)
        else:
            flash('pdfファイルではありません')
            saveFileName = ""
        return render_template('index.html', upload_img_url=saveFileName)
    else:
        return redirect('/')

def path_(a,b,c):
    d= os.path.join(a,b)
    return os.path.join(d,c)

# サーバーへPattern をアップロードする
@app.route('/pattern', methods=['GET', 'POST'])
def pattern():
    if request.method == 'POST':
        img_file = request.files['img_file']
        if img_file and img_file.filename.endswith('.png'):
            saveFileName = os.path.join(
                app.config['UPLOAD_FOLDER'], datetime.now().strftime("%Y%m%d_%H%M%S_")+secure_filename(img_file.filename))
            if os.path.isfile(saveFileName):
                flash('ファイル名が重複しています')
                saveFileName = ""
            else:
                img_file.save(saveFileName)
                conn = sqlite3.connect(app.config['DATABASE'])
                cur = conn.cursor()
                cur.execute("INSERT INTO response2(username,url,choose,circle,line) VALUES (?,?,?,?,?)",
                            (session['username'], saveFileName, 1 , 10 , 6 ))
                conn.commit()
                conn.close()
        else:
            flash('pngファイルではありません')
            saveFileName = ""
        return redirect('/')
    else:
        return redirect('/')



中略

##python app nonx      --->  Format for Non-X without Fxx/Sxx
##python app           --->  Format for X with Fxx/Sxx

if __name__ == '__main__':
    app.config['USE_FxxSxx_NOTATION'] =  True
    app.config['PORT']='8080'
    app.config['DATABASE']='members.db'
    app.config['ADMIN_PASS']="xxxx"  ## X-USER

    if len(sys.argv)>=2:
        if sys.argv[1]=="nonx":
            app.config['USE_FxxSxx_NOTATION'] = False
            app.config['PORT'] = '8888'
            app.config['DATABASE']='members2.db'
            app.config['ADMIN_PASS']="xxxx"  ## NONX-USER
    app.run('localhost', port=app.config['PORT'] ,debug=True)
  • 気になる質問をクリップする

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

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

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • dodox86

    2020/02/14 10:12

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

    キャンセル

  • siruku6

    2020/02/14 11:50 編集

    リファクタリングで動作が変わる恐れがあるということであれば、自分でリファクタリングするよりは、unittestだけ実装してあげてリファクタリングは引き継いだ相手にしてもらった方が安全でかつ、将来にわたってプラスになるような気がしました。

    実際にリファクタリングで動かなくなるリスクも大幅に減らせます。

    ただし、周囲の理解が得られるかはわかりません、、、

    スマホしか現在使えず、ソース全体を見渡すのが難しいのでひとまずこれだけお伝えしてみます。

    キャンセル

回答 4

+5

ご提示のコードは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 14:10 編集

    保守する側の立場からも、「リファクタリング」の観点からも、高評価させていただきました。

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

    キャンセル

+2

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

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

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

0

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

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

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

check解決した方法

-2

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

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

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2020/04/25 08:02

    最低の自己解決。

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

    つまり、報告もなく、連絡もせず、相談はひっくり返している。

    キャンセル

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

  • ただいまの回答率 87.49%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る