var res = 7;
var area = [];
var camP = [random(500,1000), random(500,1000)];
var setArea = function()
{
area = [];
for (var y = -200; y < 200; y+=res)
{
var row = [];
for (var x = -200; x < 200; x+=res)
{
row.push(noise((x+camP[0])/200, (y+camP[1])/200));
}
area.push(row);
row = [];
}
};
var drawPix = function(v, x, y){
if (v < 0.49)
{
fill(random(30,40), random(100,128), random(20,30));
}
else
{
fill(random(0,10), random(40,50), random(245,255));
}
rect(x*res,y*res,res,res);
};
var drawArea = function()
{
background(255, 255, 255);
noStroke();
for (var y in area)
{
for (var x in area[y])
{
drawPix(area[y][x],x,y);
}
}
};
setArea();
drawArea();
下面是输出示例：
#1 楼
欢迎使用代码审查！我在这里可以谈论很多事情。首先，您的变量名需要更具描述性。所有这些变量应使用更具描述性的名称：
var res = 7;
var area = [];
var camP = [random(500,1000), random(500,1000)];
area = [];
for (var y = -200; y < 200; y+=res)
for (var x = -200; x < 200; x+=res)
var drawPix = function(v, x, y){
最好不要使用任何语言的单字母变量名称，因为它们无助于理解代码正在做。为变量提供适当的名称可能非常困难。这是编程的最大挑战之一，但要正确执行也是重要的挑战。
您的方法名称也可以更明确。他们应该尽可能简明地描述他们将要做什么。因此这些方法名称：
var setArea = function()
var drawPix = function(v, x, y){
var drawArea = function()
应更改为以下名称：
var createMap = function()
var drawOnePixel = function(v, x, y){
var drawMap = function()
这些名称可能并不完美，但它们更接近预期。
经过编辑以反映以下评论：
与浮点数的比较很好：
if (v < 0.49)
但是，您应该知道，如果尝试直接比较浮点数，例如
if (v == 0.49)
if (v != 0.49)
您可能会遇到问题。关于此原因的更好更冗长的描述可以在这里找到：使用==比较浮点数有什么问题？
还请参见下面的注释以获取更多信息。
我必须解决这个问题的一个主意是，对所有类似的事情都使用整数，并避免使用浮点数。我不确定这是否总体上是更好的做法，但这是需要考虑的事情。如果您这样做
if (v == 49)或if (v == 490)，那么您永远都不会遇到这个问题。这是一个好习惯，并且可以很好地分离关注点。加油！评论
\ $ \ begingroup \ $
关于比较浮点值，通常仅在比较相等（==和！=）时才会出现。使用小于/大于（<和>）进行比较不会出现相同的问题。即使对于“或等于”变体，添加阈值也一样，因为它只是移动目标杆（x <= 5.0 || abs（x-5.0）<=阈值与x <= 5.0 +阈值相同） 。尽管其余所有内容都为+1，但总体而言，这还是一件好事。
\ $ \ endgroup \ $
– David Harkness
2014-09-17 23:15
\ $ \ begingroup \ $
您应该删除有关比较浮点数的部分，因为这是错误的。任何和所有语言。
\ $ \ endgroup \ $
– slebetman
2014年9月18日下午13:00
\ $ \ begingroup \ $
我将很快编辑我的答案。
\ $ \ endgroup \ $
–巴佐拉
2014-09-18 13:22
\ $ \ begingroup \ $
尽管我总体上同意您关于单字母变量的说法非常糟糕（例如上面的v），但在这里使用x和y似乎是非常合适的。 （当然，只要它们继续代表笛卡尔空间中的位置即可。）
\ $ \ endgroup \ $
–枫
2014-09-18 14:42
\ $ \ begingroup \ $
必须在这里同意Maple。 V并不可怕，因为将v用于方差是很常见的，但是它可能更明确。但是，当涉及到x和y时，仅仅为了“没有1个字母名称”而放置xCoord或yCoord似乎很愚蠢。与int相同，我是一个简单的for循环。当然，您可以使用更具描述性的内容吗，但是我使用int告诉我“这是一个简单的for循环”-它实际上传达了很多信息，而更正式的名称可能不行。
\ $ \ endgroup \ $
– corsiKa
2014年9月18日下午16:02
#2 楼
做得很好！我要选择的一件事是，您的代码中充满了幻数：var camP = [random(500,1000), random(500,1000)];
// ...
for (var y = -200; y < 200; y+=res)
// ...
for (var x = -200; x < 200; x+=res)
// ...
row.push(noise((x+camP[0])/200, (y+camP[1])/200));
// ...
if (v < 0.49)
// ...
fill(random(30,40), random(100,128), random(20,30));
// ...
fill(random(0,10), random(40,50), random(245,255));
将这些数字放入已定义的变量中在顶部。
通过这样做，您将有效地给它们命名，
可以解释它们的含义。
特别推荐您反复使用的值，例如200.
当一个值依赖于另一个值（从另一个值派生）时，该关系可以在定义中表示。例如，参数对中的差异
random(0,10), random(40,50), random(245,255)可疑是10，可能是有原因的，这可能会在变量定义中变得很清楚。 > var FILL_RGB_RANGE = 10;
var FILL_R = 0;
var FILL_G = 40;
var FILL_B = 245;
fill(
random(FILL_R, FILL_R + FILL_RGB_RANGE),
random(FILL_G, FILL_G + FILL_RGB_RANGE),
random(FILL_B, FILL_B + FILL_RGB_RANGE)
);
评论
\ $ \ begingroup \ $
我认为要填充的三个参数是RGB值。 v确定是地砖（应为绿色）还是海洋砖（应为蓝色）。
\ $ \ endgroup \ $
–justhalf
2014-09-18 6:29
\ $ \ begingroup \ $
感谢@justhalf，我将它们重命名为更有意义。不过，这很重要，这只是具有常量的示例，这些常量可以互相重用以定义关系，从而更容易理解代码的作用和原因。如果我看到两个数字40、50，则它们的含义和关系不清楚，不可见，您可以而且应该通过写为FILL_G和FILL_G + RANGE来阐明
\ $ \ endgroup \ $
– janos
2014-09-18 7:33
\ $ \ begingroup \ $
是的，我同意。实际上，您的困惑恰好表明了良好的变量命名的必要性=）
\ $ \ endgroup \ $
–justhalf
2014-09-18 7:39
#3 楼
我注意到了几件快速的事情：您已经两次声明了area []，一次是全局声明，一次是在
for（... in ...）用于遍历对象而不是数组，请看一下forEach（）
多点注释会很好，尽管可以通过更好的属性命名和使用来否定
在函数声明中使用括号时要保持一致，无论是否将它们与函数放在同一行（我不敢说首选项，Sorry Doug）。
通过jshint.com运行它总是很有帮助的。
评论
是否可以链接到网站上的实际问题描述？@DavidHarkness你到底是什么意思？
您可以链接到描述随机地形生成任务的页面吗？
@DavidHarkness哦，这不是要在网站上解决的问题或实际问题。您可以在此处在Khanacademy上创建程序。
后续问题在这里