[PATCH conductor] BZ 790516 - Only accept ipv4, hostname addresses for instances

Matt Wagner matt.wagner at redhat.com
Wed Feb 15 01:01:59 UTC 2012


On Tue, Feb 14, 2012 at 07:29:49PM -0500, Matt Wagner wrote:
>  
> +# Extract 'ipv4' and 'hostname' addresses from Deltacloud
> +def extract_addresses(address_list)
> +  addresses = []
> +  address_list.each do |address|
> +    addresses << address[:address] if ['ipv4', 'hostname'].include?(address[:type])
> +  end
> +  addresses
> +end

By the way, slightly outside the scope of a normal patch email, but I'm
curious -- can anyone see a way to refactor the above method to not use
the "addresses" array without incurring an extra array iteration? I
rewrote this a few ways, and this was the least-bad, but I still feel
like there's something better out there.

I initially wrote it as a one-liner:

 address_list.select{|a| ['ipv4', 'hostname'].include?(a[:type])}.
  collect{|a| a[:address]}

(Well, that is "one line" wrapped to 2 in my email client.)

But this was really ugly to read, and it required iterating through two
arrays instead of one. (Granted, it's not likely to ever be more than a
handful of elements, but still...)

I then rewrote it as the version that made it into the patch above. I
tried to take it a step further and eliminate the "addresses" array, so
that it was just a block:

  address_list.collect do |address|
    address[:address] if ['ipv4', 'hostname'].include?(address[:type])
  end

That's the code I really wanted, but it doesn't quite work -- "collect"
will capture a nil wherever the conditional in the block is false.
Replacing the last line with "end.compact" feels like a cheap hack, plus
it ultimately causes another array iteration.

Does anyone have a cleaner solution?

-- Matt



More information about the aeolus-devel mailing list