6
\$\begingroup\$

Can this code be made more like idiomatic ruby using methods such as map or inject?

@random_tour is a variable length array of points on a graph: [[0, 0], [2, 1], [4, 3]]

I want to calculate the distance to reach each point using the manhattan distance (number of moves horizontally + number of moves vertically).

For example, starting at [0, 0] - the next point [2, 1] is 2 moves horizontally, 1 move vertically which is 3 moves. The next point is [4, 3] which is 4 moves away from the last. Total number of moves is 7.

def cost
  i = 0
  total_cost = 0
  while i < @random_tour.length - 1 do
    total_cost += (@random_tour[i + 1][0] - @random_tour[i][0]).abs + (@random_tour[i + 1][1] - @random_tour[i][1]).abs
    i += 1
  end
  total_cost
end
\$\endgroup\$
  • 2
    \$\begingroup\$ Just to post an annoying comment: you might use a different or larger set as an example to show that negative change in distance is still counted as a positive (otherwise you're just subtracting first from last). \$\endgroup\$ – Barry Carter Apr 15 '16 at 15:06
4
\$\begingroup\$

Enumerable.each_cons(n) will pass the array items in groups of n, using a sliding window. So [pt1, pt2], [pt2, pt3], [pt3, pt4], etc.

After that you can use inject to calculate the distance and sum the points.

def manhattan(pt1, pt2)
  (pt1[0]-pt2[0]).abs + (pt1[1]-pt2[1]).abs
end

def cost
  @random_tour
    .each_cons(2)
    .inject(0) { |sum, pts| sum + manhattan(pts[0], pts[1]) }
end
\$\endgroup\$
  • \$\begingroup\$ +1 for each_cons – it's a little-known, often useful method which saves so much time. \$\endgroup\$ – Nic Hartley Apr 15 '16 at 18:15
  • 1
    \$\begingroup\$ Note that you can unpack the array in the block: { |sum, (x, y)| sum + manhattan(x, y) }. \$\endgroup\$ – tokland Apr 15 '16 at 20:23
  • 1
    \$\begingroup\$ I noticed that in your code. I had attempted to do it originally, but didn't know the correct syntax. Thanks \$\endgroup\$ – Zack Apr 15 '16 at 20:24
4
\$\begingroup\$

The problem with your code is that is too imperative, it's based on indexes, accumulators and in-place updates. Instead, a more declarative tries to apply transformations to immutable streams of data.

@Zack has already pointed you to each_cons(2) to get consecutive pair-wise combinations. I'd write it slightly different, though: I'd isolate the calculation of Manhattan distances and the total sum. You may use lazy enumerables if the intermediate array happens to be a problem:

def cost
  @random_tour
    .each_cons(2)
    .map { |(x1, y1), (x2, y2)| (x2 - x1).abs + (y2 - y1).abs }
    .reduce(0, :+)
end
\$\endgroup\$
  • \$\begingroup\$ I like your solution. Extracting the distance function like Zack did, could make it an one-liner - @random_tour.each_cons(2).map(&:manhattan_distance).reduce(0, :+). \$\endgroup\$ – sschmeck Apr 17 '16 at 18:16
  • 1
    \$\begingroup\$ Indeed, but that you'd have to add manhattan_distance to Array for that to work. \$\endgroup\$ – tokland Apr 17 '16 at 18:49
  • \$\begingroup\$ Or, alternatively, say .map(&method(:manhattan_distance)) \$\endgroup\$ – Jörg W Mittag Apr 27 '16 at 13:16
0
\$\begingroup\$

Only one tip for you: zip.

Whenever you see code that's exactly the same thing, but an index changes, zip (with a map and inject) could be useful. For example, in your situation, you'd replace this bit:

(@random_tour[i + 1][0] - @random_tour[i][0]).abs + (@random_tour[i + 1][1] - @random_tour[i][1]).abs

See how all that changes is the last index? You can change this easily, without really reducing readability, to this:

@random_tour[i + 1].zip(@random_tour[i]).map { |(end, start)| (end - start).abs }.inject(:+)

The benefit to this approach is that if you want to change the operation you're doing to the pairs, you only change it once; ditto for the places you're getting the pairs and the way you put them together. In addition, if you decide to change this to a 3D random walk, it'll Just Work® without you having to do a thing, because it automatically loops through every dimension.

If you want it to be even more expandable, try inject(:-) inside the map -- that is, |p| p.inject(:-) inside map's block. In this case, it doesn't really make sense, but if you ever... I dunno, have more than one distance? Okay, yeah, it really doesn't make sense. I just like inject's magic.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.