7

私はJS webappクライアントを書いています。ユーザーは、テキスト項目のリスト/ツリー (todo リストやメモなど) を編集できます。jQuery で DOM をよく操作します。

ユーザーは、キーボード (GMail の J/K キーに似ています) を使用してリストを上下に移動し、その他のいくつかの操作を実行できます。これらの操作の多くには、「アップ」/「ダウン」機能があります。

$.fn.moveItemUp = function() {
    var prev = this.getPreviousItem();
    prev && this.insertBefore(prev);
    // there's a bit more code in here, but the idea is pretty simple,
    // i.e. move the item up if there's a previous item in the list
}

$.fn.moveItemDown = function() {
    var next = this.getNextItem();
    next && this.insertAfter(next);
    // ....
}

2 つのほぼ同一の関数を持つこのパターンは、私のコードのいくつかの場所で繰り返されます。これは、アイテムのリスト/ツリーにかなり対称的な操作が多数あるためです。

質問:コードの重複を避けるために、これをエレガントにリファクタリングするにはどうすればよいですか?

これまでに思いついた簡単な方法は、.apply() を使用することです...

$.fn.moveItem = function(direction) {
        var up = direction === 'up',
            sibling = up ? this.getPreviousItem() : this.getNextItem(), 
            func = up ? $.fn.insertBefore : $.fn.insertAfter;

        // ...

        if (! sibling) { return false; }

        func.apply(this, [ sibling ]);

        // ...
    };

コードの他の要素の構造で moveUp/moveDown を変更する必要がある場合、メンテナンスが容易になるという利点があります。コードを何度も少し変更する必要がありましたが、常に2か所で行う必要があることを覚えておく必要があります...

しかし、次の理由から、「統合」バージョンには満足していません。

  • 実装は単純な moveUp/moveDown よりも複雑であるため、将来的には保守が容易でなくなる可能性があります。シンプルなコードが好きです。これらすべてのパラメーター、三項演算、.apply() は、「上昇」および「下降」するはずのすべての関数のトリック...
  • jQuery.fn.* 関数を明示的に参照することが安全かどうかはわかりません (結局のところ、それらは jQuery オブジェクト $('...').* に適用して使用することになっています)。

DOM または同様の構造を使用する場合、これらの「ほぼ同一のコード」の状況をどのように解決しますか?

4

3 に答える 3

4

できることの 1 つは、クロージャーを使用して、構成パラメーターの受け渡しをユーザーから隠すことです。

var mk_mover = function(direction){
     return function(){
         //stuff
     };
};

var move_up = mk_mover("up");
var move_down = mk_mover("down");

この方向に進みたい場合は、基本的に、パラメータを共通の実装に渡す最善の方法を決定することが問題になり、これに対する唯一の最善の解決策はありません。

可能な方向の 1 つは、OO スタイルのアプローチを使用して、構成の「戦略」オブジェクトを実装関数に渡すことです。

var mk_mover = function(args){
    var get_item = args.getItem,
        ins_next = args.insNext;
    return function(){
        var next = get_item.call(this);
        next && ins_next.call(this, next);
    };
 };

 var move_up = mk_mover({
     get_item : function(){ return this.getPrevious(); },
     get_next : function(x){ return this.insertBefore(x); }
 });

 var move_down = mk_mover({/**/});

戦略インターフェイス (方向によって異なるメソッド) が小さく、比較的一定であり、将来的に新しい種類の方向を追加する可能性を切り開きたい場合は、これを行うことを好みます。

また、JS は switch ステートメントよりも OO をより適切にサポートしているため、方向のセットもメソッドの種類もあまり変更する必要がないことがわかっている場合にも、これを使用する傾向があります。

他に考えられる方向は、使用した列挙型アプローチです。

var mk_mover = function(direction){
    return function(){
        var next = (
            direction === 'up' ? this.getNextItem() :
            direction === 'down' ? this.getPreviousItem() :
            null );

        if(next){
            if(direction === 'up'){
                this.insertAfter(next);
            }else if(direction === 'down'){
                this.insertBefore(next);
            }
        }

        //...
    };
}

場合によっては、switch ステートメントの代わりにきちんとしたオブジェクトまたは配列を使用して、物事をよりきれいにすることができます。

var something = ({
    up : 17,
    down: 42
}[direction]);

これはちょっと不器用だと告白しなければなりませんが、一連の方向性が早い段階で固定されていれば、必要に応じて新しい if-else または switch ステートメントを柔軟に追加できるという利点があります。自己完結型の方法 (戦略オブジェクトにいくつかのメソッドを別の場所に追加する必要はありません...)

ところで、あなたが提案したアプローチは、私が提案した 2 つのバージョンの中間にあると思います。

渡された値方向タグに応じて関数内で切り替えを行っているだけの場合は、必要な場所で直接切り替えを行う方がよいでしょう。物事を別の関数にリファクタリングしたり、煩わ​​しい .call を何度も実行したりする必要はありません。そして.applyのもの。

一方、関数の定義と分離に苦労した場合は、手動でディスパッチする代わりに、(戦略オブジェクトの形式で) 呼び出し元から直接受け取るようにすることができます。

于 2012-05-14T18:22:16.453 に答える
2

私は以前に同様の問題に遭遇したことがありますが、一見似たようなタスクを逆に実行するために見つけた優れたパターンは実際にはありません。

それらが似ていることは人間としては理にかなっていますが、コンパイラに関する限り、実際には任意のメソッドを呼び出しているだけです。唯一の明らかな重複は、呼び出される順序です。

  1. 前後に挿入したいアイテムを見つける
  2. アイテムが存在することを確認します
  3. その前後に現在のアイテムを挿入する

基本的にこれらのチェックを行う 1 つの方法で問題を解決しました。ただし、ソリューションは少し読みにくいです。これを解決する方法は次のとおりです。

$.fn.moveItem = function(dir){
  var index = dir == 'up' ? 0:1,
      item = this['getPreviousItem','getNextItem'][index]();
  if(item.length){
    this['insertBefore','insertAfter'][index].call(this,item);
  }
}

配列インデックスを使用することで、例よりも少し簡単に呼び出されているときに何が呼び出されているかを確認できます。また、渡す引数の数がわかっているため、apply を使用する必要はありません。そのため、そのユース ケースには call の方が適しています。

それがより良いかどうかはわかりませんが、少し短く (return false が必要ですか?)、もう少し読みやすい IMO です。

私の意見では、アクションを一元化することは、読みやすさよりもわずかに重要です。何かを移動するアクションを最適化、変更、または追加する場合は、1 つの場所で行うだけで済みます。読みやすさの問題を解決するために、いつでもコメントを追加できます。

于 2012-05-14T18:05:12.137 に答える
1

ワンファンクションのアプローチが一番いいと思いますが、それでもその上下のものから切り離します。アイテムをインデックスまたは他のアイテムの前に移動する1つの関数を作成し（後者の方が望ましい）、方向に関係なく、そこですべての「移動」イベントを一般的に処理します。

function moveBefore(node) {
    this.parentNode.insertBefore(this, node); // node may be null, then it equals append()

    // do everything else you need to handle when moving items
}
function moveUp() {
    var prev = this.previousSibling;
    return !!prev && moveBefore.call(this, prev);
}
function moveDown() {
    var next = this.nextSibling;
    return !!next && moveBefore.call(this, next.nextSibling);
}
于 2012-05-14T18:11:00.103 に答える