To/#163 adapt geo utils #175
Conversation
# Conflicts: # src/main/scala/edu/ie3/util/osm/OsmModel.scala # src/main/scala/edu/ie3/util/osm/TagUtils.scala
| * @return | ||
| * a [[LineString]] between the provided points | ||
| */ | ||
| def buildSafeLineStringBetweenPoints(p1: Point, p2: Point): LineString = { |
There was a problem hiding this comment.
Here, points are expected as arguments and then their underlying coordinates are extracted
There was a problem hiding this comment.
Actually, it might still make sense to use points here, since a point is a geometry consisting of one coordinate
There was a problem hiding this comment.
Would like to foster this aspect. You could easily forward to buildSafeLineStringBetweenCoordinates and would avoid double logic. You might as well check, if then buildSafePoint still is needed at all.
There was a problem hiding this comment.
You are right! I just copied the old code in that case and didn't notice!
| import scala.math.pow | ||
| import scala.util.{Failure, Success, Try} | ||
|
|
||
| object GeoUtils { |
There was a problem hiding this comment.
Remember to re-introduce function signatures that are used in the old java classes
There was a problem hiding this comment.
As discussed since we get rid of osmonauts data structures and we would have to reimplement the java code in scala we chose in that case to just get rid of it
ckittl
requested review from
ckittl,
johanneshiry,
larslenssen and
sebastian-peter
| import scala.math.pow | ||
| import scala.util.{Failure, Success, Try} | ||
|
|
||
| object GeoUtils { |
|
Does it fully address #81 or only partially? |
|
Additionally: Would you mind adding a method to calculate the center of a way (doesn't have to be closed). For an example please refer to: |
Co-authored-by: Chris Kittl <[email protected]>
Co-authored-by: Chris Kittl <[email protected]>
# Conflicts: # CHANGELOG.md
This comment has been minimized.
This comment has been minimized.
I think the GeoUtils now are quite manageable in terms of size and content. Nevertheless one could argue to separate them further into something like |
# Conflicts: # CHANGELOG.md # src/main/java/edu/ie3/util/geo/GeoUtils.java # src/test/groovy/edu/ie3/util/geo/GeoUtilsTest.groovy
This comment has been minimized.
This comment has been minimized.
|
!test |
|
!test |
This comment has been minimized.
This comment has been minimized.
|
!test |
This comment has been minimized.
This comment has been minimized.
|
Analysis Details0 Issues
Coverage and Duplications
Project ID: edu.ie3:utils |
Codecov Report
@@ Coverage Diff @@
## master #175 +/- ##
=============================================
+ Coverage 44.66% 59.13% +14.46%
+ Complexity 223 195 -28
=============================================
Files 27 25 -2
Lines 1350 876 -474
Branches 177 107 -70
=============================================
- Hits 603 518 -85
+ Misses 717 334 -383
+ Partials 30 24 -6 Continue to review full report at Codecov.
|
Resolves #163
Resolves #161
Partly adresses #81 (by moving some functionality to the RichGeometries)