72516


## 問題

JavaScriptを使ってwebアプリを作っているんですが、リファクタリングしたコードでイベントが発生するたびに毎回newする箇所があって、少し気になります。普段エンジニアのみなさんはこのような書き方をしないと伺いました。どのように直したら良いでしょうか？  
ご指導お願いします。

## リファクタリング後（リファクタリング前にはなかったimgRGBが追加されています。）

[code]

    class callPythonFnc {
        async getTitle(input, dlcard) {
            let title = await eel.get_title(input.value)();
            dlcard.querySelector('.dlcard__title > .dlcard__videoName').innerText = title;
        }
        async getSrc(videoID, thumbnailIMG) {
            let url = await eel.get_src(videoID)();
            thumbnailIMG.src = url;
        }
    
    }
    
    class thumbnail {
        constructor(){
            this.DOM = {}
            this.DOM.inputURL = document.querySelectorAll('.dlcard__url > input');
            this.DOM.dlcardWrap = document.querySelector(".dlcardWrap");
            this._addEvent();
        }
    
        getVideoID(input) {
            if (input.value.match(/^https:\/\/www\.youtube\.com\/watch\?v=.*/)) {
                const url = new URL(input.value)
                //urlの?以降の値を取り出す。
                let pairs = url.search.substring(1).split('&');
                let params = {}
                for(let pair of pairs) {
                    let kv = pair.split('=');
                    //それぞれをpairで辞書型格納する。
                    params[kv[0]] = kv[1];
                }
                let videoID = params.v;
                return videoID;
            }else{
                return false;
            }
        }
    
        imgRGB(thumbnailIMG) {
            console.log(thumbnailIMG.target);
            const colorThief = new ColorThief();
            const thumColor = colorThief.getColor(thumbnailIMG.target);
            console.log(thumColor);
        }
    
        changeTitleThumbnail(elm) {
            const input = elm.target;
            if (!input.parentElement.classList.contains("dlcard__url")) {
                console.log('dlcard_urlが取得出来ていないです。')
            }else{
                let videoID = this.getVideoID(input);
                let dlcard = input.closest('.dlcard');
                let thumbnailIMG = dlcard.childNodes[3].childNodes[1];
                thumbnailIMG.addEventListener("load", this.imgRGB, true);
                if(videoID) {
                    new callPythonFnc().getTitle(input, dlcard)
                    new callPythonFnc().getSrc(videoID, thumbnailIMG);
                }else{
                    input.value = '';
                    input.placeholder = 'URLが違います。';
                }
            }
        }
    
        _addEvent() {
            this.params = {}
            this.DOM.dlcardWrap.addEventListener("blur", this.changeTitleThumbnail.bind(this), true);
        }
    }
    
    new thumbnail();
    
[/code]

## リファクタリング前

[code]

    class thumbnail {
        constructor(){
            
            this.DOM = {}
            this.DOM.inputURL = document.querySelectorAll('.dlcard__url > input');
            this.DOM.dlcardWrap = document.querySelector(".dlcardWrap");
            this._addEvent();
        }
    
    
        _addEvent() {
            this.params = {}
            this.DOM.dlcardWrap.addEventListener("blur", 
                function (elm) {
                    const input = elm.target;
                    if (input.parentElement.classList.contains("dlcard__url")) {
                        if (input.value.match(/^https:\/\/www\.youtube\.com\/watch\?v=.*/)) {
                            const url = new URL(input.value)
                            let pairs = url.search.substring(1).split('&');
                            let params = {}
                            for(let pair of pairs) {
                                let kv = pair.split('=');
                                params[kv[0]] = kv[1];
                            }
                            let videoID = params.v;
                            let dlcard = input.closest('.dlcard');
                            let thumbnailIMG = dlcard.childNodes[3].childNodes[1];
                            if(videoID) {
                                async function run() {
                                    let title = await eel.get_title(input.value)();
                                    dlcard.querySelector('.dlcard__title > .dlcard__videoName').innerText = title;
                                }
                                run();
    
                                async function getSrc() {
                                    let url = await eel.get_src(videoID)();
                                    thumbnailIMG.src = url;
                                }
                                getSrc();
                                
                                
                            }
                        }else{
                            input.value = '';
                            input.placeholder = 'URLが違います。';
                        }
                    }else{
                        console.log('dlcard_urlが取得出来ていないです。')
                    }
                }, true);
        }
    }
    
    new thumbnail();
    
[/code]


CC BY-SA 4.0
2020-12-08T23:06:51.577
72508
2020-12-09T04:02:42.860
22565
question
javascript
JavaScriptでリファクタリングしたコードでイベントが走るたびにnewする箇所が気になる。


`callPythonFnc` のような何のデータも持たないクラスは通常は定義しません。  
(例外: ある関数がインスタンスを受け取ってその特定のメソッドを呼び出すことになっている場合は、データメンバがなくてもクラスを定義する必要がある)

クラスではないけどある分野の関数をまとめたい場合は名前空間を作るかJavaScriptモジュールにします。JavaScriptモジュールのほうが今風ですが、ローカルファイルでは使えないなどの制約があり、eel
環境で使えるかどうかはわかりません。

### 名前空間

[code]

    const python = {};
    python.getTitle = async function(input, dlcard) { ... };
    python.getSrc = async function(videoId, img) { ... };
    
    // 呼び出し例
    python.getTitle(input, dlcard);
    
[/code]

### JavaScriptモジュール

**python.mjs**

[code]

    export async function getTitle(input, dlcard) { ... }
    export async function getSrc(videoId, img) { ... }
    
[/code]

**呼び出し側**

[code]

    import * as python from 'python.mjs';
    
    ...
    python.getTitle(input, dlcard);
    
[/code]

または

[code]

    import {getTitle, getSrc} from 'python.mjs';
    
    ...
    getTitle(input, dlcard);
    
[/code]

* * *

提示されているコードの場合、私ならeelの呼び出し関連をわざわざ別にまとめたりしないで、`thumbnail`のメソッドにするか関数を消してしまうと思います。現在のコードの`getTitle()`と`getSrc()`はDOMの更新の処理も入っていて、責任範囲の分離ができていません。

あとついでですが、

[code]

                //urlの?以降の値を取り出す。
                let pairs = url.search.substring(1).split('&');
                let params = {}
                for(let pair of pairs) {
                    let kv = pair.split('=');
                    //それぞれをpairで辞書型格納する。
                    params[kv[0]] = kv[1];
                }
                let videoID = params.v;
                return videoID;
    
[/code]

は

[code]

                return url.searchParams.get('v');
    
[/code]

でよいのではないでしょうか。


CC BY-SA 4.0
2020-12-09T03:57:22.220
72516
2020-12-09T04:02:42.860
2020-12-09T04:02:42.860
3475
3475
72508
answer