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

Tomas Sedovic tsedovic at redhat.com
Wed Feb 15 16:32:58 UTC 2012


On 02/15/2012 02:01 AM, Matt Wagner wrote:
> 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.
>

Reduce/Inject would work here, but since Ruby isn't a proper functional 
language, reduce is much less usable for producing compound structures 
(arrays, strings, hashes).

address_list.reduce([]) do |arr, address|
   if ['ipv4', 'hostname'].include?(address[:type])
     arr + [address[:address]]
   else
     arr
   end
end

This would run the block for each item in the original array only once. 
Ruby arrays being mutable, though, every `arr + [...]` statement 
probably copies the entire array before adding the new element.

And it doesn't read that nice either.

As Martyn suggested, you could write your own "filtered map/collect" 
method, but really, it hardly seems like the way to go here.

> Does anyone have a cleaner solution?
>
> -- Matt


-- 
No trees were killed to send this message, but a large number of 
electrons were terribly inconvenienced.



More information about the aeolus-devel mailing list