0

I have multiple concurrent conditions in which the order is dependent on the conditions. I've come up with multiple approaches to deal with this issue and I need help choosing the best possible solution out of several that I have prepared. Things to consider are: Performance, Re-usability, Readability, Memory Consumption.

Approach 1: define multiple nested switch cases

var collision = {
    detect1: function(subject, target){
        // multiple switch cases
        var shapes = {};
        shapes[subject.type] = subject;
        shapes[target.type] = target;

        switch(subject.type) {
            case 'rectangle':
                switch(target.type) {
                    case 'ellipse':
                        return this.rectWithEllipse(subject, target);
                }
                break;
            case 'ellipse':
                switch(target.type) {
                    case 'rectangle':
                        return this.rectWithEllipse(target, subject);
                }
        }
    },

Approach 2: Store types in an object registry and switch order based on parameter testing

    detect2: function(subject, target){
        // object registry and place switch
        var shapes = {};
        shapes[subject.type] = subject;
        shapes[target.type] = target;

        var shape1 = subject;
        var shape2 = target;

        var reverseShapeOrder = function() {
            shape2 = target;
            shape1 = subject;
        };

        if ( shapes.rectangle && shapes.ellipse ) {
            if (subject.type === 'ellipse') {
                reverseShapeOrder();
                return this.rectWithEllipse(shape1, shape2);
            }
        }
    },

Approach 3: concatenate types to a string and switch order based on indexOf testing for order.

    detect3: function(subject, target) {
        // string concat and decoding with place switch
        var shapeString = subject.type + target.type;

        var rectIndex =  shapeString.indexOf('rectangle');
        var ellipseIndex = shapeString.indexOf('ellipse');
        var pointIndex = shapeString.indexOf('point');

        var shape1 = subject;
        var shape2 = target;

        var reverseShapeOrder = function() {
            shape2 = target;
            shape1 = subject;
        };

        if (rectIndex && ellipseIndex) {
            if (ellipseIndex < rectIndex) {
                reverseShapeOrder();
            }
            return this.rectWithEllipse(shape1, shape2);
        }
    },

Approach 4: Standard traditional if-else statements

    // traditional logic
    detect4: function(subject, target) {
        if (subject.type === 'rectangle'  && target.type === 'ellipse') {
            return this.rectWithEllipse(subject, target);
        }
        else if (subject.type ==='ellipse' && target.type === 'rectangle') {
            return this.rectWithEllipse(target, subject);
        }
    },
    rectWithEllipse: function(rect, ellipse) {
        return false;
    }
};   

Approach 5: On the fly selector with reference function (thanks for the on-the-fly selector suggestion @Bergi)

detect5: function(subject, target) {
        return this[subject.type + '_with_' + target.type](subject, target);
    },
    rect_with_ellipse: function(rect, ellipse) {
        return false;
    },
    ellipse_with_rect: function(rect, ellipse) {
        this.rect_with_ellipse(ellipse, rect);
    }
};   

Please help me select the best solution and understand why it is best. Thanks

Keep in mind that the full list of combinations will be larger like so:

rectWithPoint: function(rect, point) {
    return false;
},
rectWithEllipse: function(rect, ellipse) {
    return false;
},
rectWithRect: function(rect, rect) {
    return false;
},
ellipseWithPoint: function(ellipse, point) {
    return false;
},
ellipseWithEllipse: function(ellipse, ellipse) {
    return false;
}
yosefrow
  • 2,128
  • 20
  • 29
  • Approach five: Just call `this[subject.type + "With" + target.type](target, subject)` (possibly with some name-fiddling) – Bergi Jun 30 '16 at 23:31
  • Side question: is this code intended to be used as-is, or passed through Google Closure Compiler or a similar tool? – Arnauld Jun 30 '16 at 23:41
  • The code will be used as is for now. But eventually I'd want to compile all my code as a matter of course. @Arnauld – yosefrow Jun 30 '16 at 23:49
  • @Bergi see what I've added – yosefrow Jun 30 '16 at 23:49
  • Assuming Google Closure Compiler in ADVANCED_OPTIMIZATIONS mode, I would consider using integers instead of strings for all the types, using declarations such as `/** @const */ var ELLIPSE = 0;`to preserve readability. That would allow you to do things such as `switch(type0 | (type1 << 4)) { case ELLIPSE | (RECTANGLE << 4): ... }`, where the case value will be replaced by a single integer in the compiled source. – Arnauld Jul 01 '16 at 00:03
  • @Arnauld Very nice idea man. So basically, I can run detection on the second variable, because its shifted 4 to the left. But why specifically shift it by 4? Also, i assume this is meant to be used with a nested switch case – yosefrow Jul 01 '16 at 00:08
  • The shift by 4 was just an example, assuming up to 16 different possible types. It's meant for a single switch(), since it allows you to test two types at once (or even more, as long as the final mask fits in a 32-bit integer). – Arnauld Jul 01 '16 at 00:16
  • @Arnauld fair enough. A bit mask is a nice idea actually. And I hadnt thought of shifting to indicate the value of the second variable. But there still remains the issue of entering the variables in the correct order. Would you suggest setting up proxy functions to do that, or a mapping such as was shown in the selected answer, or just a nested switch case? – yosefrow Jul 01 '16 at 00:19
  • 1
    There are plenty of ways of doing it, really, and I'm not sure which one is the best. You could do `switch(type0 < type1 ? type0 | (type1 << 4) : type1 | (type0 << 4))` to make sure that the type with the lowest ID goes into the least significant bits. Or you could just put two consecutive 'case' for both combinations, followed by their common code block. – Arnauld Jul 01 '16 at 00:25
  • @Arnauld wow man, nice solution. I probably won't use this because of readability issues. But in a performance sensitive situation this would be super-awesome. Here take some geek-points :D – yosefrow Jul 01 '16 at 00:33

1 Answers1

3

I would define a mapping as follows, which will make it easy to extend:

targets: {
  rectangle: {
    point: function rectWithPoint(rect, point) {
      return false;
    },
    ellipse: function rectWithEllipse(rect, ellipse) {
      return false;
    },
    rectangle: function rectWithRect(rectLhs, rectRhs) {
      return false;
    }
  },
  ellipse: {
    point: function ellipseWithPoint(ellipse, point) {
      return false;
    },
    ellipse: function ellipseWithEllipse(ellipseLhs, ellipseRhs) {
      return false;
    }
  }
},

detect5: function (subject, target) {
  var tmp, candidates = this.targets[subject];

  if (!candidates) {
    tmp = subject;
    subject = target;
    target = tmp;
    candidates = this.targets[subject];
  }

  if (candidates && candidates.hasOwnProperty(target)) {
    return candidates[target].call(this, subject, target);
  }

  return false;
}
Dylon
  • 1,730
  • 15
  • 14
  • This is an interesting approach I had not thought of. I'm going to take a bit of time to consider it. What are the disadvantages and advantages of this approach over the other ones? – yosefrow Jun 30 '16 at 23:51
  • 1
    To add a new collision detector, you only need to define one pair of keys: (subject -> target) -> predicate. This will keep your code DRY, simple, and concise as you support many-more shapes. It's a common approach to modeling graphs, of which this is a simple kind. – Dylon Jun 30 '16 at 23:57
  • In addition, I would say that this is far and away the most readable approach – Anthony Rivas Jun 30 '16 at 23:58
  • I'm still curious to hear what other people have to say. But this is pretty sound reasoning. Thanks alot :) @Dylon – yosefrow Jul 01 '16 at 00:00
  • @Dylon what does the Rhs and Lhs stand for btw ? – yosefrow Jul 01 '16 at 00:21
  • Right-hand-side and left-hand-side, respectively. They are traditional, parameter names for binary operations. For example, lhs + rhs, lhs * rhs, etc. – Dylon Jul 01 '16 at 00:24
  • @yosefrow: It's pretty much similar to my suggestion. Whether flattened or not shouldn't make a huge difference, it's definitely the right way to go. – Bergi Jul 01 '16 at 00:47
  • @Bergi yeah. I particularly like the way this is implemented, as it allows for really easy and readable extension. Flat might work for just a few. But in this case where there will be many cases, I think a map like this is really the way to go. It also helps keep track of which cases you've covered already. – yosefrow Jul 01 '16 at 00:50