5
\$\begingroup\$

In this task I had to create simple IP address / subnet calculator in Python. I'm just wondering how you see this problem.

There is my code:

def toBinary(integer):
    binary = ['{0:0>8}'.format(bin(int(x))[2:]) for x in integer.split(".")]
    return binary

def ip_information(ip, mask):
    binary_ip = toBinary(ip)
    binary_mask = toBinary(mask)
    network_ip = ["", "", "", ""]
    broadcast_address = ["", "", "", ""]
    number_of_hosts = 1

    for x in range(4):
        for y in range(8):
            network_ip[x] += str(int(binary_ip[x][y]) and int(binary_mask[x][y]))
            broadcast_address[x] += str(int(not int(binary_mask[x][y])))
            if binary_mask[x][y] == '0':
                number_of_hosts *= 2

        network_ip[x] = int(network_ip[x], 2)
        broadcast_address[x] = int(broadcast_address[x], 2) + network_ip[x]

    return f"""
    Network IP: {".".join(str(x) for x in network_ip)}
    Broadcast address: {".".join(str(x) for x in broadcast_address)}
    Number of hosts: {number_of_hosts - 2}
    """

Output:

    Network IP: 192.168.0.0
    Broadcast address: 192.168.0.255
    Number of hosts: 254

Any tips on how to make this better, closer to an advanced (but still, can take under account to make it simple)? Or just a better solution will be definitely on point.

Using Python 3.10.

\$\endgroup\$
3
  • 4
    \$\begingroup\$ Are you aware of ipaddress? \$\endgroup\$ Commented Jan 12, 2022 at 2:30
  • \$\begingroup\$ Yes, but I have to make it calculations on my own. \$\endgroup\$ Commented Jan 12, 2022 at 15:17
  • \$\begingroup\$ Your function gives a wrong answer for a 31-bit network prefix, as well as for all IPv6 addresses. For example, ip_information('172.23.45.67', 31) should return a network of 172.23.45.66 (or no network at all, depending on how you define it), a broadcast of 172.23.45.67 (or no broadcast at all, depending on how you define it), and 2 hosts. \$\endgroup\$ Commented Jan 12, 2022 at 20:03

1 Answer 1

9
\$\begingroup\$

PEP8

You should use snake_case for function names. You did so with ip_information, why not with to_binary as well?

Naming

The function name ip_information does not convey what it precisely does. You might want to rename it.

Docstrings

Both of your functions are excellent candidates for docstrings to describe what they are supposed to be doing.

Use existing library functions

Since you did not tag reinventing-the-wheel, I suppose that you just want to get the job done. If so, your entire code can be replaced with the usage of the ipaddress module from the standard library:

from ipaddress import IPv4Network


def print_network_information(ipv4network: IPv4Network) -> None:
    """Prints the network address, broadcast address and number
    of addresses on the given IPv4 network.
    """

    print('Network IP:', ipv4network.network_address)
    print('Broadcast address:', ipv4network.broadcast_address)
    print('Number of hosts:', ipv4network.num_addresses)


def main() -> None:
    
    print_network_information(IPv4Network('192.168.0.0/255.255.255.0'))


if __name__ == '__main__':
    main()

Note that the number of hosts differs from your version's number, since the network address and broadcast address are also valid host addresses within CIDR. If you want to compensate for this, just subtract 2 from the respective value.

\$\endgroup\$
0

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.