Skip to content

Commit

Permalink
Reduce code duplication in elevation_add() for #37
Browse files Browse the repository at this point in the history
  • Loading branch information
Robinlovelace committed Sep 3, 2021
1 parent b153e6a commit 6ba560d
Showing 1 changed file with 5 additions and 12 deletions.
17 changes: 5 additions & 12 deletions R/slopes.R
Original file line number Diff line number Diff line change
Expand Up @@ -421,18 +421,11 @@ elevation_add = function(
m_xyz = cbind(m[, 1:2], z)
}
n = nrow(routes)

if(n == 1) {
rgeom3d_line = sf::st_linestring(m_xyz)
rgeom3d_sfc = sf::st_sfc(rgeom3d_line, crs = sf::st_crs(routes))
sf::st_geometry(routes) = rgeom3d_sfc
} else {
linestrings = lapply(seq(n), function(i){
rgeom3d_line = sf::st_linestring(m_xyz[m[, 3] == i, ])
})
rgeom3d_sfc = sf::st_sfc(linestrings, crs = sf::st_crs(routes))
sf::st_geometry(routes) = rgeom3d_sfc
}
linestrings = lapply(seq(n), function(i){
rgeom3d_line = sf::st_linestring(m_xyz[m[, 3] == i, ])
})
rgeom3d_sfc = sf::st_sfc(linestrings, crs = sf::st_crs(routes))
sf::st_geometry(routes) = rgeom3d_sfc
routes
}

Expand Down

4 comments on commit 6ba560d

@Robinlovelace
Copy link
Member Author

Choose a reason for hiding this comment

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

Heads-up @ateucher, this was my attempt at refactoring to dedupe code. There is more that could be done, tried a few other things below the null call above but didn't seem to do much... If you see any quick wins please let me know, great to have more eyes on it and apologies for the unsolicited ping!

@ateucher
Copy link
Member

Choose a reason for hiding this comment

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

Looks great to me @Robinlovelace - much cleaner!

One small nit - in line 425 (rgeom3d_line = sf::st_linestring(m_xyz[m[, 3] == i, ])), assigning to rgeom3d_line is unnecessary (and I would have thought it meant that the anonymous function wouldn't return anything - but it does - strange!).

@Robinlovelace
Copy link
Member Author

Choose a reason for hiding this comment

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

One small nit - in line 425 (rgeom3d_line = sf::st_linestring(m_xyz[m[, 3] == i, ])), assigning to rgeom3d_line is unnecessary (and I would have thought it meant that the anonymous function wouldn't return anything - but it does - strange!).

Oh yeah, seems obvious now, not sure how I missed that. Anonymous functions ahoy!

@Robinlovelace
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed here, many thanks @ateucher ! 20e8e90

Please sign in to comment.