リファクタリング的な作業（６）

（Python学習初心者の試行錯誤・備忘録です）
しばらく「機能追加」をしてきて、だんだんと「なんか違うな」という引っ掛かりを感じ始めました。それというのは

のメインのコードで

    #while True　ループの中、抜粋    
    elif event =="-BTN_MEMORIZE-": #記憶定着OKなら。Levelを一つ上げ、タイムスタンプ更新,表示データ更新
        mycontroller.inc_level()
        mycontroller.pickupcard(0)
        mycontroller.refresh_view(myview.window)

このように、コントローラーのクラスのメンバ関数として、refresh_view という関数を書いていること。コントローラーのクラスでは

def refresh_view(self,window):
        if self.mycards.topcard is not None:
            window["-DISP_MAIN-"].update(self.mycards.topcard['face'])
            window["-DISP_SUB-"].update("")
            window["-DISP_SUMMARY-"].update("レベル0:{}個 レベル1:{}個 レベル2:{}個 レベル3:{}個 合計 {}個"\
                                    .format(
                                        self.mycards.select_count_where("level = 0"),
                                        self.mycards.select_count_where("level = 1"),
                                        self.mycards.select_count_where("level = 2"),
                                        self.mycards.select_count_where("level = 3"),
                                        self.mycards.select_count_where("level >= 0")))

のように引数に myview.window を呼び出している。深く依存してしまっているのが（動いたとしても）気持ち悪い。
　コントローラーのクラスの「あるべき姿」としては、画面に表示すべきデータのセットを準備して、「送り出す」ところまでであって特定のVIEWやwindow変数を前提としてそれに依存するような書き方をしてはいけないのではないかと。
　MVCモデルを目標にしている以上、それぞれの役割ごとに機能を分けるべきで、コントローラークラスのrefresh_view というのはViewのクラスの方に書くべき関数でしょう。

　そう思い立って、次のように変更しました。
Controllerのクラスで

    def prepare_view(self, myview, poolnum:int = 5, picklevel :int =1):
        #L1プールの処理をして、指定レベルでカードピックアップ
        self.poolL1(poolnum)
        self.pickupcard(picklevel)
        #self.refresh_view(myview)

下準備の関数からrefresh_view(myview)　を外しました。

ViewのクラスにControllerクラスのrefresh_viewにあった内容表示更新の処理を移しました。

    def refresh(self,dbname:str, levelcount:list, viewvalue:dict):
        #dbname の反映
        self.window["-TEXT_DBNAME-"].update(dbname)
        #levelcountの反映
        self.showgraph(levelcount[0],levelcount[1],levelcount[2],levelcount[3],levelcount[4])
        self.window["-DISP_SUMMARY-"].update("STOCK:{}個 レベル1:{}個 レベル2:{}個 レベル3:{}個 合計 {}個"\
            .format(levelcount[0],levelcount[1],levelcount[2],levelcount[3],levelcount[4]))
        #viewvalue (topcard)の反映
        if viewvalue is not None:
            #self.window
            self.window["-DISP_MAIN-"].update(viewvalue['face'])
            self.window["-DISP_SUB-"].update("")
        else:
            self.window["-DISP_MAIN-"].update("None")
            self.window["-DISP_SUB-"].update("None")    

表示に必要な情報は、文字列、リスト、辞書型で受け取り、特定のクラスに依存しないようにしました。
　メインのコードの中では

myview = View()
myctrl = Controller()

def refresh_view(myview:View, myctrl:Controller, poolnum:int, picklevel: int):
    myctrl.prepare_view(myview,poolnum,picklevel)
    myview.refresh(myctrl.mycards.mydb, myctrl.levelcount, myctrl.mycards.topcard)

表示に必要な情報を、コントローラーのprepare_view関数で用意して、
ビューのmyview.refresh関数で、用意したデータを表示する。これをひとまとめで、「メイン」のコードでののrefresh_view という関数にしました。

あとは、メインのイベント処理（while Trueループ）の中で何かデータの変更があるたびにこの関数を呼び出す。という形に変更しました。

　　#抜粋
    elif event =="-BTN_MEMORIZE-": #記憶定着OKなら。Levelを一つ上げ、タイムスタンプ更新,表示データ更新
        myctrl.inc_level()
        refresh_view(myview, myctrl, value["-SPIN_POOL-"],value["-LEVEL-"])          

    elif event =="-BTN_FORGET-": #記憶定着NGなら。Levelを1に戻し、タイムスタンプ更新,表示データ更新
        myctrl.one_level()  #元zero_levelだったがレベル1にするように仕様変更
        refresh_view(myview, myctrl, value["-SPIN_POOL-"],value["-LEVEL-"]) 

また少し見通しが良くなったと自分では思います。