Eliminate duplicated code and unnecessary comments with a helper property or method

[ericlippert]

Your code for rectangles looks good -- but any time I see a comment like this

return other.left >= this.left &&
               (other.left + other.width) <= (this.left + this.width) &&  //right side

I am in the habit of asking myself "is there a way to write the code so that it does not require the comment?"

What I might do here is add getters to the rectangle class:

    get right() 
    {
        return this.left + this.width;
    }

and similarly for bottom. The commented code then becomes:

return other.left >= this.left && other.right <= this.right;

which is much easier to read and does not require a comment to explain what it is doing.

More generally: there is no computation that is too small to warrant its own function as long as you can concisely describe the semantic meaning of the function. I love small functions. They make the program easier to read and more obviously correct.

Same thing for your randomness code:

let width = Math.floor(Math.random() * 5) + 4; // random 4 to 8

That's a perfect candidate for a helper method, or even two:

// Return a random int between 0 and max
function uniformRandom(max) 
{
    return Math.floor(Math.random() * (max + 1));
}

// return a random int between min and max
function boundedRandom(min, max)
{
    return uniformRandom(max - min) + min;
}

and then your code becomes

let height = boundedRandom(4, 8); 
let left = uniformRandom(gridWidth - width + 1); // random 0 to 50 minus the width (so it doesn't extend out)

Notice that the second comment stays in. Compare that comment to the previous one that I deleted. random 4 to 8 says what the code does -- the problem is that the code was complicated enough that it was hard to figure out what it does. If code is so complicated that you can't figure out what it does, rewrite it using helper functions until it is so clear that it does not need a comment. But so it doesn't extend out explains why the code is doing what it does. Comments should be explaining the why, not the what.

---

Also, I love that you're using nested functions to extract individual tasks to their own methods, but you have correctly identified that this makes a very long outer function:

//  v  just so i dont get confused this ends the createtherooms function
    } // <------
// ^

Your instinct to put a helper comment in here to signpost the meaning of the close curly is good, and I do the same thing, but I would be a lot less verbose:

} // end createRooms

But again I would ask myself if the method was so complicated that it deserved to be in a helper class of its own, and make the nested functions into member functions.