current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite 
def dec_to_bin(ip):

    ip_array = ip.split(".")
    ip_array = filter(None, ip_array)

    if len(ip_array) != 4:
        return "Invalid IP Address format"

    else:
        ip_bin = []

        for x in range(len(ip_array)):

            # Formatting example referenced from: 
            # http://stackoverflow.com/a/10411108/1170681

            ip_bin.append('{0:08b}'.format(int(ip_array[x])))
            ip_bin.append(".")

        ip_bin.pop()
        return ''.join(ip_bin)

This is a simple parser that will take a IP address in decimal form and convert it into its binary representation.

I'm looking for any tips on coding styles and improving the efficiency of the code.

share|improve this question
    
Beware: This will not work for IPv6. –  luiscubal Dec 8 '12 at 19:16
    
@luiscubal: this solution supports both ipv4 and ipv6 addresses –  J.F. Sebastian Nov 9 '13 at 11:44
    
How does split('.') and append(".") for addresses that use : instead of .? –  luiscubal Nov 9 '13 at 14:54
    
@luiscubal: click the link –  J.F. Sebastian Nov 9 '13 at 16:54
    
@J.F.Sebastian I've read that answer and I still don't understand how. If nothing else, because your answer is very different from the code in the question. ::1.split('.') (a valid IPv6 address) returns ['::1']. The len is 1, not 4, so it returns "Invalid IP Address format". –  luiscubal Nov 9 '13 at 22:28

3 Answers 3

up vote 5 down vote accepted

Code simplification and related:

Line 4: remove line 3 and replace line 4 with

ip_array = filter(None, ip.split('.'))

Line 7: Create an exception, to prevent that a caller gets a string of error instead of the actual result.

Line 12: assuming this is not python3, it's better if you use xrange() instead of range(), because range would create an actual list listing all the items in specified range. xrange(), instead, returns the next item in the sequence without creating the list. This behaviour is the same as the range() function in python3. Anyway, for your purposes changing it with:

for el in ip_array:
    ip_bin.append('{0:08b}'.format(int(el)))

or, even better:

ip_bin = ['{0:08b}'.format(int(el)) for el in ip_array]

Last line: instead of appending the "." after each element, join over the dot:

return '.'.join(ip_bin)

Style: Pursue consistency in all the code: if you use double-quotes for defining strings, use them everywhere, same thing with single-quotes (which i personally prefer for strings, reserving double ones for docstrings).

Reassuming:

def dec_to_bin(ip):

    ip_array = filter(None, ip.split('.'))

    if len(ip_array) != 4:
        raise NotValidIPException('Invalid IP Address format.')
    else:
        ip_bin = ['{0:08b}'.format(int(el)) for el in ip_array]
        return '.'.join(ip_bin)
share|improve this answer
up vote 3 down vote 
def dec_to_bin(ip):

    ip_array = ip.split(".")
    ip_array = filter(None, ip_array)

Why do you need to filter it?

    if len(ip_array) != 4:
        return "Invalid IP Address format"

Never return strings to indicate errors, raise an exception

    else:
        ip_bin = []

        for x in range(len(ip_array)):

Use for ip_element in ip_array:, you should almost never have to iterate over range(len(...)).

            # Formatting example referenced from: 
            # http://stackoverflow.com/a/10411108/1170681

             ip_bin.append('{0:08b}'.format(int(ip_array[x])))

What happens if the user put something else besides numbers in there? I'd suggest matching the ip address against a regular expression to make it more robust.

            ip_bin.append(".")

        ip_bin.pop()
        return ''.join(ip_bin)

Instead use '.'join(ip_bin) and don't try putting the '.' in the list. It'll be simpler.

share|improve this answer
up vote 0 down vote

In Python 3.3:

import ipaddress

dec_to_bin = lambda ip: bin(int(ipaddress.ip_address(ip)))

It supports both ip4 and ip6.

On older Python versions, you could use socket and struct modules. To convert ipv4 address:

import socket
import struct

dec_to_bin = lambda ip4: bin(struct.unpack('!I', socket.inet_pton(socket.AF_INET, ip4))[0])

Example:

>>> dec_to_bin('192.168.1.1')
'0b11000000101010000000000100000001'

To convert ipv6 address:

import socket
import struct

def int_from_ipv6(addr):
    hi, lo = struct.unpack('!QQ', socket.inet_pton(socket.AF_INET6, addr))
    return (hi << 64) | lo

dec_to_bin = lambda ip6: bin(int_from_ipv6(ip6))

Example:

>>> dec_to_bin("2002:c0a8:0101::").rstrip('0')
'0b1000000000001011000000101010000000000100000001'
share|improve this answer
    
Why are you defining it as a lambda instead of using def? –  Winston Ewert Nov 13 '13 at 1:19
    
no reason. It is slightly more convenient to work with in a plain REPL. –  J.F. Sebastian Nov 13 '13 at 1:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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