Readability of two rectangular point function samples I have two implementations of the same function. Which of these is easier to understand?

Code sample A

function makeRectanglePoint(box, index){

    var boxToUse;

    function randomPoint(a, b){
        return Math.random() * Math.abs(a - b) + Math.min(b, a)
    }

    if(index % 2){ boxToUse = box.p1 } else { boxToUse = box.p2 }

    if(index &lt; 2){
        return new Point(randomPoint(box.p1.x, box.p2.x), boxToUse.y);
    } else {
        return new Point(boxToUse.x, randomPoint(box.p1.y, box.p2.y));
    }

}


Code sample B

function makeRectanglePoint(rectangle, pointIndex){

    function getRectangleEdge(){

        function getPointByIndex(index) {
            switch (index) {
                case 0: return rectangle.p1;
                case 1: return new Point(rectangle.p2.x, rectangle.p1.y);
                case 2: return rectangle.p2;
                case 3: return new Point(rectangle.p1.x, rectangle.p2.y);
            }
        }

        function Line(p1, p2){
            this.p1 = p1;
            this.p2 = p2;
        }

        var endPointIndex = (pointIndex + 1) % 4;
        return new Line(
            getPointByIndex(pointIndex),
            getPointByIndex(endPointIndex)
        );
    }

    function generateRandomPointOnEdge(line){

        function generateRandomBetween(a, b){
            return Math.min(a, b) + Math.random() * Math.abs(a - b);
        }

        return new Point(
            generateRandomBetween(line.p1.x, line.p2.x),
            generateRandomBetween(line.p1.y, line.p2.y)
        );

    }

    return generateRandomPointOnEdge(getRectangleEdge());
}
I'd say version A wins simply by being more succinct. Version B does however provide some extra detail, such as the fact that the point being added is being added on an edge (not inside a rectangle). Still, the structure of version A is a lot more straightforward to read.

Both version could use comments though!

I'd also replace the 1-line if..else with a simple ternary:

boxToUse = index % 2 ? box.p1 : box.p2;


Speaking of, shouldn't that be pointToUse instead of boxToUse? In either case, though, I'd ask "to use for what exactly?"

I don't know what this is used for, but I imagine a function that returns a random number in a given range would be useful elsewhere, so you might consider extracting it. Regardless, its name shouldn't be randomPoint, since it doesn't return a point - it just returns a number.

As for version B (just to give it a once over):


Using a constructor for Line is overkill. Just use an object literal: {p1: p1, p2: p2}
The switch is tricky. It has no default case and instead relies on its input being 0-3




All in all it's hard to say much more, since I don't know the context. 

But a 3rd way might be better still. I'd recommend something like

// Returns a point lying on one of the rectangle's edges
// The edge is given as a number:
// 0: top edge
// 1: right edge
// 2: bottom edge
// 3: left edge
function makeRectanglePoint(rect, edge) {
  // generate a random number within the rect's height or length
  var rand = edge % 2 ? random(rect.p1.y, rect.p2.y) : random(rect.p1.x, rect.p2.x);

  switch(edge % 4) {
    case 0: return new Point(rand, rect.p1.y); // top
    case 2: return new Point(rand, rect.p2.y); // bottom
    case 1: return new Point(rect.p2.x, rand); // right
    case 3: return new Point(rect.p1.x, rand); // left
  }

  // handle errors (i.e. edge not being an integer or being negative)
  throw new Error("Edge index should be an integer between 0 and 3 (inclusive)";
}