JavaScriptリファクタリング実践１

JavaScriptパターン ―優れたアプリケーションのための作法を読みながら先日紹介したBeatubeのソースをリファクタリングしました。

リファクタリングの例として説明する処理は、曲やシリーズ名の情報をとあるサイトから取得してDBに格納するというものです。この処理をcronモジュールで定期的に実行させます。

リファクタリング前

var request = require('request');
var cheerio = require('cheerio');

var db = require('./lib/db.js');
var Musics = db.Musics;
var Series = db.Series;

// コレクションの全データを削除
function reset_data(){
    Musics.remove({},function(error){
        if(error) return console.log(error);
    });

    Series.remove({},function(error){
        if(error) return console.log(error);
    });
}

// 全曲表からデータ取得
function get_data(){
    var requestUrl = 'http://iidxsd.sift-swift.net/view/all';
    request({url: requestUrl}, function(error, response, body) {
        if(!error && response.statusCode == 200){
            var $ = cheerio.load(body);
            var series_count = 0;

            $("#datatable .data_title tr td h3").each(function(){
                var series = new Series();
                series.series = series_count;
                series.title = $(this).text();
                series.save(function(error){
                    if(error) return console.log(error);
                });
                
                console.log(series);
                series_count++;
            });
            console.log("get series finished.");

            series_count = 0;
            $("#datatable .data_body").each(function(){
                $(this).children("tr").each(function(){

                    var music = new Musics();
                    music.series = series_count;
                    music.title = $(this).children("td").first().text().replace(/^詳/,"");
                    music.genre = $(this).find(".genre").text();
                    music.artist = $(this).children(".artist").text();
                    music.bpm = $(this).children(".bpm").text();
                    music.dif_n7 = zeroCheck($(this).children(".difn7").text());
                    music.dif_h7 = zeroCheck($(this).children(".difh7").text());
                    music.dif_a7 = zeroCheck($(this).children(".difa7").text());
                    music.dif_n14 = zeroCheck($(this).children(".difn14").text());
                    music.dif_h14 = zeroCheck($(this).children(".difh14").text());
                    music.dif_a14 = zeroCheck($(this).children(".difa14").text());
                    music.notes_n7 = zeroCheck($(this).children(".notesn7").text());
                    music.notes_h7 = zeroCheck($(this).children(".notesh7").text());
                    music.notes_a7 = zeroCheck($(this).children(".notesa7").text());
                    music.notes_n14 = zeroCheck($(this).children(".notesn14").text());
                    music.notes_h14 = zeroCheck($(this).children(".notesh14").text());
                    music.notes_a14 = zeroCheck($(this).children(".notesa14").text());
                    music.save(function(error){
                        if(error) return console.log(error);
                    });

                    console.log(music);
                });
                series_count++;
            });
            console.log("get musics finished.");
        }
        else{
            if(error) return console.log(error);
        }
    });
};

function zeroCheck(value){
    return value.match(/[^0-9]+/) ? 0 : value;
}

exports.scraping = function(){
    reset_data();
    get_data();
};

var scraping = require('./scraping.js');

// cron設定 ==========================
var cronJob = require('cron').CronJob;
var cronTime = "00 00 00 */7 * *";    // 7日間隔で実行
// var cronTime = "00 */1 * * * *";  // 1分間隔で実行

job = new cronJob({
  cronTime: cronTime

    // The function to fire at the specified time.
    , onTick: function() {
        scraping.scraping();
        //console.log(new Date());
    }

  // Specified whether to start the job after just before exiting the constructor.
  , start: false
}).start();

問題点として、以下のことが上げられます。

データ取得とDB格納が一つの関数に混在しており、可読性やメンテナンス製が悪い。
また、上記の理由により単体テストがしにくい。
変数や関数が散らかってる。

リファクタリング１回目

var SCRAPING = {
    request: require('request'),
    cheerio: require('cheerio'),

    SCRAPING_REQUEST_URL: 'http://iidxsd.sift-swift.net/view/all',

    getSeries: function(callback){
        var cheerio = this.cheerio;
        var series_count = 0;
        var series = [];

        this._getBody(function(error, body){
            if(error) return callback(series);

            var $ = cheerio.load(body);
            $("#datatable .data_title tr td h3").each(function(){
                var s = {
                    series: series_count,
                    title: $(this).text()
                };
                series.push(s);
                series_count++;
            });
            return callback(series);
        });
    },

    getMusics: function(callback){
        var cheerio = this.cheerio;
        var zeroCheck = this._zeroCheck;
        var series_count = 0;
        var musics = [];

        this._getBody(function(error, body){
            if(error) return callback(musics);

            var $ = cheerio.load(body);
            $("#datatable .data_body").each(function(){
                $(this).children("tr").each(function(){
                    var m = {
                        series: series_count,
                        title: $(this).children("td").first().text().replace(/^詳/,""),
                        genre: $(this).find(".genre").text(),
                        artist: $(this).children(".artist").text(),
                        bpm: $(this).children(".bpm").text(),
                        dif_n7: zeroCheck($(this).children(".difn7").text()),
                        dif_h7: zeroCheck($(this).children(".difh7").text()),
                        dif_a7: zeroCheck($(this).children(".difa7").text()),
                        dif_n14: zeroCheck($(this).children(".difn14").text()),
                        dif_h14: zeroCheck($(this).children(".difh14").text()),
                        dif_a14: zeroCheck($(this).children(".difa14").text()),
                        notes_n7: zeroCheck($(this).children(".notesn7").text()),
                        notes_h7: zeroCheck($(this).children(".notesh7").text()),
                        notes_a7: zeroCheck($(this).children(".notesa7").text()),
                        notes_n14: zeroCheck($(this).children(".notesn14").text()),
                        notes_h14: zeroCheck($(this).children(".notesh14").text()),
                        notes_a14: zeroCheck($(this).children(".notesa14").text())
                    };
                    musics.push(m);
                });
                series_count++;
            });
            return callback(musics);
        });
    },
    /**
   * リクエストのbody要素を返却する
   * 
   * @method _getBody
   * @return {String} body要素
   */
    _getBody: function(callback){
        this.request({url: this.SCRAPING_REQUEST_URL}, function(error, response, body) {
            if(error || response.statusCode != 200){
                // console.log(error);
                return callback(error);
            }
            // console.log(body);
            return callback(null, body);
        });
    },


    _zeroCheck: function(value){
        return value.match(/[^0-9]+/) ? 0 : value;
    }
};

exports.scraping = SCRAPING;

var scraping = require('./scraping').scraping;
var db = require('./db');
var cronJob = require('cron').CronJob;

var cron_time = "00 00 00 */7 * *";   // 7日間隔で実行
// var cron_time = "*/10 * * * * *"; // 10秒間隔で実行(debug)

var job = new cronJob({
    cronTime: cron_time,

    // The function to fire at the specified time.
    onTick: function() {
        console.log("cron start");
        // seriesのinsert。データが既にある時はupdate。
        scraping.getSeries(function(series){
            var i, max;
            for(i = 0, max = series.length; i < max; i++){
                db.Series.update({number: series[i].number}, series[i], {upsert: true}, function(error, numberAffected, raw){
                    if(error) console.log(error);
                });
            }
        });

        // musicsのinsert。データが既にある時はupdate
        scraping.getMusics(function(music){
            var i, max;
            for(i = 0, max = music.length; i < max; i++){
                db.Musics.update({series_number: music[i].series_number,title: music[i].title},
                    music[i], {upsert: true}, function(error, numberAffected, raw){
                    if(error) console.log(error);
                });
            }
        });
    },

    // Specified whether to start the job after just before exiting the constructor.
    start: false
}).start();

見ての通り、スクレイピング部分をオブジェクトとしてひとまとまりにし、DB格納部分と分けました。

しかしもう少し良くできそうです。

リファクタリング２回目

/**
* 曲リストのあるサイトからデータを取得し、配列にして返す
* 
* @class Scraping
*/
exports.Scraping = function Scraping(){
    // ローカル変数
    var request = require('request');
    var cheerio = require('cheerio');
    var SCRAPING_REQUEST_URL = 'http://iidxsd.sift-swift.net/view/all';

    /**
   * シリーズの名前のリストを返す
   * 
   * @method getSeries
   * @return {Array} シリーズの名前のリスト
   */ 
    this.getSeries = function(callback){
        var series_count = 0;
        var series = [];

        _getBody(function(error, body){
            if(error) {
                callback(null);
                return [];
            }

            var $ = cheerio.load(body);
            $("#datatable .data_title tr td h3").each(function(){
                var s = {
                    series: series_count,
                    title: $(this).text()
                };
                callback(s)
                series.push(s);
                series_count++;
            });
            return series;
        });
    };

    /**
   * 曲の情報のリストを返す
   * 
   * @method getMusics
   * @return {Array} 曲の情報のリスト
   */ 
    this.getMusics = function(callback){
        var series_count = 0;
        var musics = [];

        _getBody(function(error, body){
            if(error) {
                callback(null);
                return [];
            }

            var $ = cheerio.load(body);
            $("#datatable .data_body").each(function(){
                $(this).children("tr").each(function(){
                    var m = {
                        series: series_count,
                        title: $(this).children("td").first().text().replace(/^詳/,""),
                        genre: $(this).find(".genre").text(),
                        artist: $(this).children(".artist").text(),
                        bpm: $(this).children(".bpm").text(),
                        dif_n7: _zeroCheck($(this).children(".difn7").text()),
                        dif_h7: _zeroCheck($(this).children(".difh7").text()),
                        dif_a7: _zeroCheck($(this).children(".difa7").text()),
                        dif_n14: _zeroCheck($(this).children(".difn14").text()),
                        dif_h14: _zeroCheck($(this).children(".difh14").text()),
                        dif_a14: _zeroCheck($(this).children(".difa14").text()),
                        notes_n7: _zeroCheck($(this).children(".notesn7").text()),
                        notes_h7: _zeroCheck($(this).children(".notesh7").text()),
                        notes_a7: _zeroCheck($(this).children(".notesa7").text()),
                        notes_n14: _zeroCheck($(this).children(".notesn14").text()),
                        notes_h14: _zeroCheck($(this).children(".notesh14").text()),
                        notes_a14: _zeroCheck($(this).children(".notesa14").text())
                    };
                    callback(m);
                    musics.push(m);
                });
                series_count++;
            });
            return musics;
        });
    };

    /**
   * リクエストのbody要素を返却する
   * 
   * @method _getBody
   * @return {String} body要素
   */
    function _getBody(callback){
        request({url: SCRAPING_REQUEST_URL}, function(error, response, body) {
            if(error || response.statusCode != 200){
                // console.log(error);
                return callback(error);
            }
            // console.log(body);
            return callback(null, body);
        });
    }

    /**
   * 数値でない値の場合、0にして返す
   * 
   * @method _zeroCheck
   * @param {String} value
   * @return {String} 0 または value
   */
    function _zeroCheck(value){
        return value.match(/[^0-9]+/) ? 0 : value;
    }
};

/**
* cronの設定
*/

var db = require('./db');
var Scraping = require('./scraping').Scraping;
var CronJob = require('cron').CronJob;

//var cronTime = "00 00 00 */7 * *"; // 7日間隔で実行
var cronTime = "*/10 * * * * *";  // 10秒間隔で実行(debug)

var job = new CronJob({
    cronTime: cronTime,

    // The function to fire at the specified time.
    onTick: function() {
        console.log("Cron start.");

        var scraping = new Scraping();
        
        // seriesのinsert。データが既にある時はupdate。
        scraping.getSeries(function(series){
            // console.log(series);
            db.Series.update({number: series.number}, 
                series, {upsert: true}, function(error, numberAffected, raw){
                if(error) console.log(error);
            });
        });

        // musicsのinsert。データが既にある時はupdate
        scraping.getMusics(function(music){
            // console.log(music);
            db.Musics.update({series_number: music.series_number,title: music.title},
                music, {upsert: true}, function(error, numberAffected, raw){
                if(error) console.log(error);
            });
        });
    },

    // Specified whether to start the job after just before exiting the constructor.
    start: false
}).start();