Code Reviews

# PHP script to create a KML square centred on a point.

+4
−0

The objective here is to create a set of square kml coordinates centered on a point. I use this to create a square map centered on a mountain peak and then turn that into a STL that I can 3D print.

``````<?php

echo "Enter Latitude\n";
\$handle = fopen("php://stdin", "r");
fclose(\$handle);
echo "Enter Longitude\n";
\$handle = fopen("php://stdin", "r");
fclose(\$handle);
echo "Enter Distance\n";
\$handle = fopen("php://stdin", "r");
\$meter = trim(fgets(\$handle));
fclose(\$handle);

\$kml .= "<coordinates>";

\$i = 45;
for (\$loop = 0; \$loop <= 4; \$loop += 1) {
\$lon_rad = fmod((\$long + \$dlon_rad + M_PI), 2 * M_PI) - M_PI;
\$i += 90;
}
\$kml .= "</coordinates>\n";

echo \$kml;
``````
Why does this post require moderator attention?
Why should this post be closed?

+3
−0

1 - Rearrange the code: Open stdin, read all parameters, close stdin, then process the parameters.

2 - Consider adding some validation to the parameters.

3 - Add an explanation of the magic number 6378137 - I had to Google it to find out what it means.

4 - Replace \$i = 45, \$i += 90 and the for statement with:

```for (\$i = 45; \$i <= 405; \$i += 90) {
```

Which actually doesn't look right. Is it supposed to be \$loop <= 4, which turns into \$i = 405? Or should it be < 4 which would have the last \$i = 315?

5 - Assign cos(\$d_rad), sin(\$d_rad), sin(\$lat) and cos(\$lat) to variables outside the loop instead of calculating them repeatedly.

Why does this post require moderator attention?

+2
−0
``````echo "Enter Latitude\n";
\$handle = fopen("php://stdin", "r");
fclose(\$handle);
echo "Enter Longitude\n";
\$handle = fopen("php://stdin", "r");
fclose(\$handle);
echo "Enter Distance\n";
\$handle = fopen("php://stdin", "r");
\$meter = trim(fgets(\$handle));
fclose(\$handle);
``````

I found the opening and closing of stdin quite confusing, but worse than that it fails to work in my test environment. Open it once and close it at the end.

`\$handle` is not a very descriptive name. How about `\$stdin`?

The prompt for "distance" will be more usable for other people (and perhaps yourself in two years) if you state the units. Degrees can reasonably be inferred for latitude and longitude, but there are various common units for distance. It might also be helpful to define what distance it is.

``````\$kml .= "<coordinates>";
``````

I get a "notice" complaining about using `.=` instead of `=` for a variable which hasn't previously been defined.

``````\$d_rad = \$meter / 6378137;
``````

Magic! `\$d_rad` for "distance in radians"?

``````\$i = 45;
for (\$loop = 0; \$loop <= 4; \$loop += 1) {
\$lon_rad = fmod((\$long + \$dlon_rad + M_PI), 2 * M_PI) - M_PI;
\$i += 90;
}
\$kml .= "</coordinates>\n";
echo \$kml;
``````

`\$loop++` would be more idiomatic: I think Python is probably the only language where `+= 1` is the idiomatic increment operator.

The mathematics could use multiple comments:

1. Define what you mean by "square".
2. Give a reference or derivation for the formulae.
3. Address issues of valid ranges, numerical analysis, etc. For example, the code is clearly incorrect at the poles, where the output appears to collapse to a line.

There are arguments for preferring to handle string concatenation in a loop with array concatenation followed by `implode`:

``````\$kml = ["<coordinates>"];
for (...) {
...
}
\$kml[] = "</coordinates>\n";
echo implode(\$kml);
``````

This isn't a major issue with a small fixed loop, so consider this a nit-pick.

Why does this post require moderator attention? 