Validating a Collection of IPv4s

I have a method within a class that validates all proxy addresses are valid on initialization. I’ve tried to refactor this a bit, but this seems to be the best I can do. Any suggestions and/or tips as to how I can possibly improve this method?

def valid?(proxies)
  return false if !proxies.is_a?(Array) || proxies.empty?
  ipv4_address = /^((d{1,3}[.]){3}d{1,3}:d+)$/
  proxies.each do |proxy|
    return false if proxy !~ ipv4_address
    octets = proxy.scan(/d+/).map(&:to_i).first(4)
    return false unless octets.all? { |octet| octet.between?(0, 255) }
  end
  true
end

The only simplification I can think of is using a bit more complex regex to match the IP address without having to iterate over the octets:

def valid?(proxies)
  return false if !proxies.is_a?(Array) || proxies.empty?
  ipv4_address = /^((25[0-5]|2[0-4]d|1dd|[1-9]d|d).){3}(25[0-5]|2[0-4]d|1dd|[1-9]d|d):d+$/
  proxies.each do |proxy|
    return false if proxy !~ ipv4_address
  end
  true
end

This regex is based on the one in the ipaddress package.

You might decide to add a method to Array, so that you can just call %w(4.4.4.4 8.8.8.8).ipv4?. I originally looked to use !detect but all? is much clearer for avoiding double negation. Also note that self (e.g., self.all?) is unnecessary when opening a class.

class Array
  def ipv4?
    return false if empty?
    ipv4_address = /^((25[0-5]|2[0-4]d|1dd|[1-9]d|d).){3}
                      (25[0-5]|2[0-4]d|1dd|[1-9]d|d)$/x
    all? { |address| address =~ ipv4_address }
  end
end

Ruby has an IPAddr class in the standard library. I would use IPAddr objects instead of Strings to represent IP addresses.

Trying to create an instance for an invalid IP address raises an exception, so you’ll have to handle that case. Once you’ve done that, though, you’re guaranteed to only work with valid IP addresses in the rest of you application.

proxies = proxies.map { |ip| IPAddr.new(ip) rescue false }

def valid?(proxies)
  proxies.all?(&:ipv4?)
end

Some comments:

• return false if !proxies.is_a?(Array): IMO you shouldn’t test the type of an method arguments.
• proxies.each. I’d recommend using the right functional abstraction, not the generic each. In this case: Enumerable#all?

I’d write:

def valid?(proxies)
  !proxies.empty? && proxies.all? do |proxy|
    proxy.match(Ipv4Address) && proxy.split(".").all? { |s| s.to_i.between?(0, 255) }
  end
end