Random walk terrain generator

Question

I was messing around with Javascript in Khanacademy the other day and this "monstrosity" was born. It's a terrain generator that uses a simplified version of the random walk method.

// current tile position
var BlockX = 200;
var BlockY = 200;
// tile size
var SizeX = 2;
var SizeY = 2;
// min and max X positions
var MinX = 0;
var MaxX = 400;
// min and max Y positions
var MinY = 0;
var MaxY = 400;
// possible position changes
var BlockPosChanges = [-1, 1, 0]; 
// repeats and max repeats
var MaxRepeats = round(random(10*100, 10*1250)); // Do not change! This is at the max safe amount
var Repeats = 0;
// set the background
background(0, 13, 255);
// main program loop
while(Repeats < MaxRepeats) {
 // possible color ranges
 var RedRange = round(random(45, 70));
 var BlueRange = round(random(165, 100));
 var GreenRange = round(random(20, 50));
 // draw a tile
 noStroke();
 fill(RedRange, BlueRange, GreenRange);
 rect(BlockX, BlockY, SizeX, SizeY);
 // change the current tile position
 BlockX += BlockPosChanges[floor(random() * BlockPosChanges.length)];
 BlockY += BlockPosChanges[floor(random() * BlockPosChanges.length)];
 // test if at edge
 if(BlockX <= MinX || BlockX >= MaxX) {
 break;
 }
 // test if at edge
 if(BlockY <= MinY || BlockX >= MaxY) {
 break;
 }
 // increment repeats
 Repeats++;
}

Since this version of Javascript has features that may not be included in other variants, you can test this program here in Khanacademy. What can I improve here?

• 200_success: This appears to be a follow-up to a previous question.
• David Harkness: Bug: BlockX >= MaxY should be BlockY >= MaxY.

Answer 1

Style

Variables and functions (except constructors) in JavaScript are normally written in camelCase with a leading lowercase letter. It's not mandatory, but you'll find it easier to work with others if you follow a language's conventions. There are many guides out there, but I mostly follow Google's JavaScript Style Guide.

Some of the variables do not change once set. These should use ALL_CAPS to denote that they are constants and probably assigned at the top.

Another point in the guide is to surround all operators and keywords with a space. Omitting the space between if, while, etc. and the opening parenthesis makes it harder to skim without syntax highlighting.

You can omit comments that merely restate the code in prose. At best they waste the readers time, and at worst they become misleading or incorrect when they aren't changed with the code. Good comments state programmer intent or clarify tricky algorithms. This comment adds no value.

// increment repeats
Repeats++;

Bug

As I noted in the comment, you have a typo bug:

if(BlockY <= MinY || BlockX >= MaxY) {
 ^^^^^^

This is common when using similarly-named variables, but it's difficult to avoid doing so in rendering code.

Clarity

You can improve the performance of the random choice between -1, 0, and 1--and in my opinion make it more obvious--using a simpler equation without the array:

BlockX += floor(3 * random()) - 1;

You are using a while loop where a for loop would be more obvious. As written, you have to scan the whole body to see that Repeats only gets incremented once at the end. Use this instead:

for (var Repeats = 0; Repeats < MaxRepeats; ++Repeats) { ... }