Skip to content

Feature random position#238

Open
tiinsy wants to merge 6 commits intodartclub:mainfrom
deanpapas:feature-random-position
Open

Feature random position#238
tiinsy wants to merge 6 commits intodartclub:mainfrom
deanpapas:feature-random-position

Conversation

@tiinsy
Copy link
Copy Markdown
Contributor

@tiinsy tiinsy commented May 22, 2025

implementation of randomPosition. slight differences to turfJS (turf has all random functions in one file - these are being done in separate files)

@haozeguo
Copy link
Copy Markdown

I reviewed the randomPosition implementation locally and the related tests are passing on my side. I also added validation coverage for null bbox handling.

@haozeguo
Copy link
Copy Markdown

I reviewed the randomPosition implementation locally and the related tests are passing on my side. I also added validation coverage for null bbox handling.

Ran dart format on the codebase and pushed the changes.

@hamishdgx hamishdgx requested a review from lukas-h March 31, 2026 04:45
/// ```
bool booleanDisjoint(GeoJSONObject feature1, GeoJSONObject feature2, {bool ignoreSelfIntersections = false}) {
bool booleanDisjoint(GeoJSONObject feature1, GeoJSONObject feature2,
{bool ignoreSelfIntersections = false}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can add a trailing comma after the = false,} and then it will auto-formart it for you

bool booleanDisjoint(GeoJSONObject feature1, GeoJSONObject feature2, {bool ignoreSelfIntersections = false}) {
bool booleanDisjoint(GeoJSONObject feature1, GeoJSONObject feature2,
{bool ignoreSelfIntersections = false}) {
var bool = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you should give this variable a different name to avoid a name collision with the bool type

Comment on lines +23 to +25
if (bbox.length != 4) {
throw ArgumentError("Bbox must contain exactly 4 values.");
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our bboxes could also contain 6 values if altitudes are provided

Not entirely sure if this applied to here though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be 6 if includes alts

@hamishdgx
Copy link
Copy Markdown
Contributor

@haozeguo Changes requested. Please address

@haozeguo
Copy link
Copy Markdown

haozeguo commented Apr 13, 2026 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RMITWeek1 RMIT2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants