patternjavascriptMinor
Random walk terrain generator
Viewed 0 times
randomwalkgeneratorterrain
Problem
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.
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?
// 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 = MaxX) {
break;
}
// test if at edge
if(BlockY = 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?
Solution
Style
Variables and functions (except constructors) in JavaScript are normally written in
Some of the variables do not change once set. These should use
Another point in the guide is to surround all operators and keywords with a space. Omitting the space between
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.
Bug
As I noted in the comment, you have a typo bug:
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:
You are using a
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 = 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) { ... }Code Snippets
// increment repeats
Repeats++;if(BlockY <= MinY || BlockX >= MaxY) {
^^^^^^BlockX += floor(3 * random()) - 1;for (var Repeats = 0; Repeats < MaxRepeats; ++Repeats) { ... }Context
StackExchange Code Review Q#64398, answer score: 4
Revisions (0)
No revisions yet.