3
\$\begingroup\$

I wrote this to consume valid IPv4 addresses in dotted-quad decimal form:

def dotted_ip_to_num ip
  strings = ip.split('.').reverse
  strings.each_with_index.inject(0) do |sum, pair|
    str, i = pair
    sum + str.to_i * (2 ** (i * 8))
  end
end

But my chain of each_with_index and inject (aka left-fold or reduce) looks clumsy to me.

Firstly, Ruby won't let me match (aka desctructure) the pair yielded by each_with_index like this |sum, str, i| or this |sum, [str, i]|. Did I miss something? So, I do this on a line of its own (line 4.)

Secondly, although the message chain is only 2 messages, is it so complex in this case that I should split it up to do the summation as an extra line, like this?

octets = strings.each_with_index.collect do |str, i|
  str.to_i * (2 ** (i * 8))
end
octets.inject(0, :+)  

Please set aside the improvement of multiplying the accumulator by 256 at each inject/reduce step, as that eliminates each_with_index entirely, and means I won't get the answer to my two main questions.

I also welcome reviews from users of non-Ruby collections libraries. (FYI collect is map in some other languages.)

\$\endgroup\$
0

3 Answers 3

2
\$\begingroup\$

Some notes:

  • As you say in your answer, you can unpack block arguments with the syntax |arg1, (arg2, arg3)|.

  • I'd always write parentheses in the method definition, it looks more clear (subjective, of course).

  • Instead of doing two things inside the reduce block, split it in a map + simpler reduce.

I'd write:

def dotted_ip_to_num(ip)
  xs = ip.split('.').map(&:to_i).reverse
  xs.map.with_index { |x, idx| x * (256**idx) }.reduce(0, :+)
end
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Thanks for spotting my answer at the same time is I posted my comments. I really like the way you split out the tasks so that the multiplication is in the map.with_index - and the reduce step is clearly separate. I also like that map.with_index is more readable than each_with_index.map \$\endgroup\$
    – dcorking
    Jul 22, 2014 at 11:21
  • \$\begingroup\$ What guides you to put parentheses around the method definition parameter? \$\endgroup\$
    – dcorking
    Jul 22, 2014 at 11:22
  • 1
    \$\begingroup\$ It's subjective. My understanding of seeing a lot of Ruby code is that this practice (omitting parens) is being dropped. Calls in DSL-style (Rails for example) without parens, ok. For everything else, I'd write parens. \$\endgroup\$
    – tokland
    Jul 22, 2014 at 11:23
  • 1
    \$\begingroup\$ @dcorking I second tokland's points about parentheses-usage. Some might say it's noise, but I (personally) find it easier to read. Besides, you still have to use parentheses in (chained, at least) method calls, so might as well use them in definitions. But yes, for DSLs omitting them often looks cleaner \$\endgroup\$
    – Flambino
    Jul 22, 2014 at 11:36
3
\$\begingroup\$

The correct Ruby-way (also Perl and others) is to use pack/unpack, since low-level bytes job is what they are implemented for.

def dotted_ip_to_num ip
    ip.split(".").map(&:to_i).pack("C*").unpack("N")[0]
end
\$\endgroup\$
1
0
\$\begingroup\$

I found out how to do the pattern match: use parentheses |sum, (str, i)|, thanks to a solution at Codewars. (Where is this documented?) So I can rewrite my code like this, that makes it simple enough to chain 2 message.

def dotted_ip_to_num ip
  strings = ip.split('.').reverse
  strings.each_with_index.inject(0) do |sum, (str, i)|
    sum + str.to_i * (2 ** (i * 8))
  end
end

I then went on to refactor further, so that string handling is in one method chain, and integer handling in another. I think separating types like this makes code in general easier to maintain.

def dotted_ip_to_num ip
  octets = ip.split('.').reverse.collect(&:to_i)
  octets.each_with_index.inject(0) do |sum, (octet, i)|
    sum + octet * (2 ** (i * 8))
  end
end

Finally, in this special case, we can discard the split/collect/inject entirely and use IPAddr from the Ruby standard library:

require 'ipaddr'
def dotted_ip_to_num(ip)
  IPAddr.new(ip).to_i
end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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