ふくもとのメモ帳

思いついたことや感じたこと。技術的なことを書いていきます。

オレオレリファクタリングの実例

よくある、左右にスライドしてくカルーセルと言うかスライドパネルを作ってみるとして。
右向いてのみスライドするところまで作ってみる。
slidePanel.html

<!DOCTYPE html>
<html lang="ja" dir="ltr">
    <head>
        <meta charset="utf-8">
        <script src="jquery-1.8.2.js"></script>
        <script src="jquery.easing.1.3.js"></script>
        <script>
            $(document).ready(function() {
                var pos = [];
                $('#slider ul').children().each(function() {
                    pos.push($(this).position().left);
                });
                $('#right-nav').click(function() {
                    var next = 0;
                    var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
                    var current = parseInt(marginLeft, 10) * -1;
                    $.each(pos, function() {
                        next = parseInt(this, 10);
                        if (next > (current + 30)) {
                            console.log(next + ", " + current + '+30');
                            return false;
                        }
                    });
                    $('#slider ul').animate({
                        'marginLeft': next * -1
                    },{
                        'duration': 700,
                        'easing': 'easeOutBounce'
                    });
                });
            });
        </script>
        <style>
            body
            {
                margin: 0;
                padding: 0;
            }
            #slider {
                border: 1px solid #CCC;
                width: 500px;
                overflow: auto;
            }
            #slider ul {
                list-style: none;
                width:3800px;
                margin: 0;
                padding: 0;
            }
            #slider ul li {
                background-color: #EEE;
                border: 1px solid #CCC;
                height: 100px;
                width: 500px;
                margin: 0px;
                padding: 0px;
                float: left;
            }

            #left-nav {
                position: absolute;
                left: 10px;
                top: 50px;
                text-shadow: 2px 2px 3px #000000;
            }

            #right-nav {
                position: absolute;
                left: 480px;
                top: 50px;
                text-shadow: 2px 2px 3px #000000;
            }
        </style>
    </head>
    <body>
        <span id="left-nav">＜</span>
        <span id="right-nav">＞</span>
        <div id="slider">
            <ul>
                <li>Content 1</li>
                <li>Content 2</li>
                <li>Content 3</li>
                <li>Content 4</li>
                <li>Content 5</li>
                <li>Content 6</li>
                <li>Content 7</li>
            </ul>
            <div style="clear:both;"></div>
        </div>
    </body>
</html>

ここまでのスクリーンショット
f:id:fkmt5:20130201235054p:plain
↑ここクリックしたら、右へスライドする。
f:id:fkmt5:20130201235136p:plain
↑こんな感じ。

この時に左へ戻す版も作らなくちゃいけない。
右へスライドするコードが↓こんな感じなんで、コピペして左スライドバージョンを作る。

$('#right-nav').click(function() {
    var next = 0;
    var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
    var current = parseInt(marginLeft, 10) * -1;
    $.each(pos, function() {
        next = parseInt(this, 10);
        if (next > (current + 30)) {
            console.log(next + ", " + current + '+30');
            return false;
        }
    });
    $('#slider ul').animate({
        'marginLeft': next * -1
    },{
        'duration': 700,
        'easing': 'easeOutBounce'
    });
});

↓あんまり深くは考えずにぱっぱっぱと書き換えて動かしてみる。（当然ダメなんやけど）

$('#left-nav').click(function() {
    var next = 0;
    var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
    var current = parseInt(marginLeft, 10) * -1;
    $.each(pos, function() {
        next = parseInt(this, 10);
        if (next < (current - 30)) {
            console.log(next + ", " + current + '-30');
            return false;
        }
    });
    $('#slider ul').animate({
        'marginLeft': next * -1
    },{
        'duration': 700,
        'easing': 'easeOutBounce'
    });
});

各パネルの左端と表示領域の左端を比較してるんで、左に戻る時には逆順になってなアカンのが出来てない。
ネット調べたら each を逆順に走査するやり方より先に $(pos).get().reverse() みたいな感じで配列の逆順を取り出す方法が先に見つかったんで、それで行く。スピード重要。

最終的なコードは↓こんな感じ。

$(document).ready(function() {
    var pos = [];
    $('#slider ul').children().each(function() {
        pos.push($(this).position().left);
    });
    var rpos = $(pos).get().reverse();
    $('#right-nav').click(function() {
        var next = 0;
        var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
        var current = parseInt(marginLeft, 10) * -1;
        $.each(pos, function() {
            next = parseInt(this, 10);
            if (next > (current + 30)) {
                console.log(next + ", " + current + '+30');
                return false;
            }
        });
        $('#slider ul').animate({
            'marginLeft': next * -1
        },{
            'duration': 700,
            'easing': 'easeOutBounce'
        });
    });
    $('#left-nav').click(function() {
        var next = 0;
        var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
        var current = parseInt(marginLeft, 10) * -1;
        $.each(rpos, function() {
            next = parseInt(this, 10);
            if (next < (current - 30)) {
                console.log(next + ", " + current + '-30');
                return false;
            }
        });
        $('#slider ul').animate({
            'marginLeft': next * -1
        },{
            'duration': 700,
            'easing': 'easeOutBounce'
        });
    });
});

とりあえず、コピペした奴なんで DRY 違反しまくりんぐ。それをリファクタリングで改善してく。

コピペした部分を比較して重複ロジックがどれくらいあるか測定する。
f:id:fkmt5:20130202000146p:plain
結局、違ってる部分は４行だけ。（そのうち１つはデバッグ用なので実際は３行）

違ってる部分は引数にして動的に変えるってのが一般化・共通化の第一歩なんで、pos(昇順)とrpos(降順）の配列をパラメータで渡すのは簡単。
だけど if 文をパラメータで渡すって？

if (next > (current + 30)) {
    return false;
}

今回の if 文だったら関数型言語みたいな話を持ち出すまでも無く、 C++/Java プログラマにはお馴染みの３項演算子なので２つの引数から１つ結果が返る関数に書き換えるのは簡単。

function compare(a, b) {
    if (a > (b + 30)) return false;
    return true;
}

こうなったら、重複するコードをどかんと function に切り出して、変化する部分をパラメータ化してく。

function slide(pos, compare) {
    var next = 0;
    var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
    var current = parseInt(marginLeft, 10) * -1;
    $.each(pos, function() {
        next = parseInt(this, 10);
        return compare(next, current);
    });
    $('#slider ul').animate({
        'marginLeft': next * -1
    },{
        'duration': 700,
        'easing': 'easeOutBounce'
    });
}

↑こんな感じ。その上で、呼び出し元コードは、無名関数を渡して↓こんな感じ。

var pos = [];
$('#slider ul').children().each(function() {
    pos.push($(this).position().left);
});
var rpos = $(pos).get().reverse();
$('#right-nav').click(function() {
    slide(pos, function(a, b) {
        if (a > b + 30) return false;
        return true;
    });
});
$('#left-nav').click(function() {
    slide(rpos, function(a, b) {
        if (a < b - 30) return false;
        return true;
    });
});

こんな感じになる。
このコードが平易かどうかは意見が別れるような気がする。ただ、直観的かどうかで言うと確実に直観的じゃ無いと思うのな、少なくとも自分にとっては間違いなくそう。
「if 文の大小比較で右にスライドするのか左にスライドするのか」瞬間的には読み取れない。

こういう場合、名前で追加情報をコードに与えるのが有効やと思う。意味のまとまりに名前をつけることで思考を整理するみたいな。
関数の戻り値を変数に受けなおすことで、意味が強化出来る場合は多々ある。関数名は概念をあらわす(what)けれど、それを受ける変数名は用途をあらわす（how)。言い換えるとどう使うのかって意味が強化される。
ま、こんな説明を抽象的に書いても言葉遊びになっちゃうんでプログラマはプログラマらしく、実際のコードで書くと。

<script>
    $(document).ready(function() {
        var pos = [];
        $('#slider ul').children().each(function() {
            pos.push($(this).position().left);
        });
        var rpos = $(pos).get().reverse();

        $('#right-nav').click(function() {
            slideToRight(pos);
        });
        $('#left-nav').click(function() {
            slideToLeft(rpos);
        });
    });

    function slideToLeft(pos) {
        slide(pos, function(a, b) {
            if (a < b - 30) return false;
            return true;
        });
    }

    function slideToRight(pos) {
        slide(pos, function(a, b) {
            if (a > b + 30) return false;
            return true;
        });
    }

    function slide(pos, compare) {
        var next = 0;
        var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
        var current = parseInt(marginLeft, 10) * -1;
        $.each(pos, function() {
            next = parseInt(this, 10);
            return compare(next, current);
        });
        $('#slider ul').animate({
            'marginLeft': next * -1
        },{
            'duration': 700,
            'easing': 'easeOutBounce'
        });
    }
</script>

C++やJavaなら、public と private にスコープを分けるとこやけど JavaScript にそういうのあるのかどうか知らない。
もうちょっとだけ細かいところを調整して、リファクタリングのコードは。

<!DOCTYPE html>
<html lang="ja" dir="ltr">
    <head>
        <meta charset="utf-8">
        <script src="jquery-1.8.2.js"></script>
        <script src="jquery.easing.1.3.js"></script>
        <script>
            $(document).ready(function() {
                var pos = [];
                $('#slider ul').children().each(function() {
                    pos.push($(this).position().left);
                });
                var rpos = $(pos).get().reverse();

                $('#right-nav').click(function() {
                    slideToRight(pos);
                });
                $('#left-nav').click(function() {
                    slideToLeft(rpos);
                });
            });

            function slideToLeft(pos) {
                slide(pos, function(a, b) {
                    if (a < b - 30) return false;
                    return true;
                });
            }

            function slideToRight(pos) {
                slide(pos, function(a, b) {
                    if (a > b + 30) return false;
                    return true;
                });
            }

            function slide(pos, compare) {
                var next = 0;
                var marginLeft = $('#slider ul').css('margin-left').replace("px", "");
                var current = parseInt(marginLeft, 10) * -1;
                $.each(pos, function() {
                    next = parseInt(this, 10);
                    return compare(next, current);
                });
                $('#slider ul').animate({
                    'marginLeft': next * -1
                },{
                    'duration': 700,
                    'easing': 'easeOutBounce'
                });
            }
        </script>
        <style>
            #slider {
                border: 1px solid #CCC;
                width: 500px;
                overflow: hidden;
                position: relative;
            }
            #slider ul {
                list-style: none;
                width:3800px;
                margin: 0;
                padding: 0;
            }
            #slider ul li {
                background-color: #EEE;
                border: 1px solid #CCC;
                height: 100px;
                width: 500px;
                margin: 0px;
                padding: 0px;
                float: left;
            }

            #left-nav {
                position: absolute;
                left: 10px;
                top: 50px;
                text-shadow: 2px 2px 3px #000000;
            }

            #right-nav {
                position: absolute;
                left: 480px;
                top: 50px;
                text-shadow: 2px 2px 3px #000000;
            }
        </style>
    </head>
    <body>
        <div id="slider">
            <ul> 
                <li>Content 1</li>
                <li>Content 2</li>
                <li>Content 3</li>
                <li>Content 4</li>
                <li>Content 5</li>
                <li>Content 6</li>
                <li>Content 7</li>
            </ul>
            <div style="clear:both;"></div>
        </div>
        <span id="left-nav">＜</span>
        <span id="right-nav">＞</span>
    </body>
</html>

あー長いな。もうちょっとポイントを絞って書くほうが良い。まあ、いろいろ実験していこう。