TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 100 Pull requests 4 Projects Releases 1 Activity Actions

locode: Use quadtree to find continent #830

Closed
Nesterfifa wants to merge 6 commits from Nesterfifa/frostfs-node:782-use-quadtree into master
pull from: Nesterfifa/frostfs-node:782-use-quadtree
merge into: TrueCloudLab:master
Conversation 4 Commits 6 Files changed 4 +2 -3
1 changed files with 2 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 6999993fd2 - Show all commits

5
pkg/util/locode/db/continents/geojson/calls.go
View file

@ -4,12 +4,11 @@ import (
"fmt"
"os"
"github.com/paulmach/orb/quadtree"
locodedb "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/locode/db"
"github.com/paulmach/orb"
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2023-12-12 11:16:43 +00:00
Outdated
Review
Copy link

Please, attach all non-stdlib imports in a single group (see paulmach/orb below).

Please, attach all non-stdlib imports in a single group (see `paulmach/orb` below).
Nesterfifa commented 2023-12-12 12:07:12 +00:00
Outdated
Review
Copy link

Fixed

Fixed
"github.com/paulmach/orb/geojson"
"github.com/paulmach/orb/planar"
"github.com/paulmach/orb/quadtree"
)
const continentProperty = "Continent"
@ -92,7 +91,7 @@ func (db *DB) buildQuadtree() error {
})
for _, feature := range db.features {
multiPolygon := make(orb.MultiPolygon, 0)
var multiPolygon orb.MultiPolygon
achuprov commented 2023-11-30 15:21:32 +00:00
Outdated
Review
Copy link

Is there a way to simplify this code?
cff78d77c2/pkg/util/locode/db/continents/geojson/calls.go (L94-L110)

Is there a way to simplify this code? https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/cff78d77c22b788a398a958a9a9d1f02b43b6148/pkg/util/locode/db/continents/geojson/calls.go#L94-L110
Nesterfifa commented 2023-12-12 11:08:23 +00:00
Outdated
Review
Copy link

Reduced the nesting depth to 2

Reduced the nesting depth to 2
fyrchik marked this conversation as resolved
fyrchik commented 2023-12-12 11:22:19 +00:00
Review
Copy link

What about var multiPolygon orb.MultiPolygon? make(_, 0) is usually never needed: nil acts good enough as the default value.

What about `var multiPolygon orb.MultiPolygon`? `make(_, 0)` is usually never needed: `nil` acts good enough as the default value.
Nesterfifa commented 2023-12-12 12:07:21 +00:00
Review
Copy link

Fixed

Fixed
if polygon, ok := feature.Geometry.(orb.Polygon); ok {
multiPolygon = append(multiPolygon, polygon)