CIDR Notation parser

Question

I created the following code to better understand CIDR notation. Also to learn some of the Stream features in Java8. I posted my unit tests too.

I'm interested in ways it might be improved especially by using Java8 features. I reviewed the InetAddr class and realized it uses at array of bytes instead of ints. I did not find that convenient.

package com.company;
import java.util.ArrayList;
/**
 * This class accepts a String in CIDR format in the constructor
 * It calculates IP range and has methods for isInRange and count
 */
public class Cidr {
 public IPAddress ip;
 public int mask;
 // get an array of ints given a mask (like /24 /16 /8 etc...)
 int[] getIntMaskArray(int mask){
 int[] intArray = new int[4];
 String[] maskArray = getArrayBinaryStrings(mask, "1");
 for (int i = 0; i < maskArray.length; i++) {
 intArray[i] = Integer.parseInt(maskArray[i], 2);
 }
 return intArray;
 }
 String[] getArrayBinaryStrings(int mask, String fill){
 String s = "";
 String[] binArray = new String[4];
 String notFill = "0";
 if (fill.equals("0")){
 notFill = "1";
 }
 // loop of four, since there are four array elements:
 for (int x = 0 ; x < 4; x++){
 // 8 parts of each array element:
 for(int y = 0; y<8; y++){
 if(mask > 0){
 s = s+fill;
 mask = mask -1;
 } else {
 s = s+notFill;
 }
 }
 binArray[x] = s;
 s = "";
 }
 return binArray;
 }
 public IPAddress getNetMask() throws InvalidIPException {
 // netmask is the mask converted to an IP Address:
 // can't use getArrayBinaryStrings because that returns array of binary strings!
 return new IPAddress(StringUtils.implode(getIntMaskArray(mask)));
 }
 IPAddress getAddress(){
 return this.ip;
 }
 public boolean isInRange(IPAddress ipAddress){
 return this.getHostMax().compareTo(ipAddress) > 0 && this.getHostMin().compareTo(ipAddress) < 0;
 }
 public long countHosts(){
 // host count should be hostMax - hostMin + 1
 return getHostMax().toDecimal() - getHostMin().toDecimal() + 1;
 }
 public IPAddress getHostMax(){
 // host max is one less than Broadcast
 return new IPAddress(getBroadcastAddress().toDecimal() - 1);
 }
 public IPAddress getHostMin(){
 // host min is simply adding one to the network address
 return new IPAddress(this.getNetworkAddress().toDecimal()+1);
 }
 public IPAddress getNetworkAddress(){
 String[] maskArray = getArrayBinaryStrings(mask, "1");
 return this.ip.and(getIpAddressFromBinary(maskArray));
 }
 public IPAddress getBroadcastAddress(){
 String[] invertedMaskArray = getArrayBinaryStrings(mask, "0");
 return getIpAddressFromBinary(invertedMaskArray).or(getNetworkAddress());
 }
 private IPAddress getIpAddressFromBinary(String[] maskArray) {
 String[] octetArray = new String[4];
 for (int i = 0; i < maskArray.length; i++) {
 octetArray[i] = String.valueOf(Integer.parseInt(maskArray[i], 2));
 }
 try {
 return new IPAddress(String.join(".", octetArray));
 } catch (InvalidIPException iip){
 iip.printStackTrace();
 return null;
 }
 }
 /**
 *
 * @return list of all IP addresses in network defined by cidr
 */
 public ArrayList<IPAddress> ipList(){
 ArrayList<IPAddress> result = new ArrayList<IPAddress>();
 IPAddress hostMin = getHostMin();
 long longip = hostMin.toDecimal();
 while(longip <= getHostMax().toDecimal()){
 result.add(new IPAddress(longip));
 longip++;
 }
 return result;
 }
 public Cidr(String cidr){
 //divide string into IP + mask:
 String[] ip_mask = cidr.split("/");
 try {
 this.ip = new IPAddress(ip_mask[0]);
 } catch (InvalidIPException e) {
 e.printStackTrace();
 }
 this.mask = Integer.parseInt(ip_mask[1]);
 }
}

Here is the IPAddress class:

package com.company;
import java.util.stream.Stream;
import static com.company.StringUtils.padLeft;
public class IPAddress implements Comparable {
 private int[] octets;
 public IPAddress(int[] octets) {
 this.octets = octets;
 }
 @Override
 public int compareTo(Object otherObject) {
 if (otherObject != null) {
 IPAddress secondIP = (IPAddress) otherObject;
 return (int) (this.toDecimal() - secondIP.toDecimal());
 } else {
 return -1;
 }
 }
 @Override
 public boolean equals(Object obj) {
 /* two ip addresses are equal if toStrings are equal: */
 return obj instanceof IPAddress && this.toString().equals(obj.toString());
 }
 public int[] getOctets() {
 return octets;
 }
 private boolean validIP(String ip){
 if(ip.split("\\.").length != 4){
 return false;
 }
 for (String octet : ip.split("\\.")){
 if (Integer.parseInt(octet) > 255 || Integer.parseInt(octet) < 0){
 return false;
 }
 }
 return true;
 }
 /**
 * create IP address using string
 * @param ip is an IP Address (ex: "10.10.232.133")
 */
 public IPAddress(String ip) throws InvalidIPException {
 // make sure the ip string is valid:
 // all the octets should fit into bytes
 if(!validIP(ip)){
 throw new InvalidIPException();
 }
 this.octets = Stream.of(ip.split("\\.")).mapToInt(Integer::parseInt).toArray();
 }
 /**
 * create IPAddress object using a decimal number
 * @param decimalRep decimal representation of IP address
 */
 public IPAddress(long decimalRep){
 String binString = padLeft(Long.toBinaryString(decimalRep),32,"0");
 // divide 32 character string into byte array:
 int[] octets = new int[4];
 int index = 0;
 int counter = 0;
 String newBinString;
 while (index < binString.length()){
 newBinString = binString.substring(index,index + 8);
 index = index + 8;
 int b = Integer.parseInt(newBinString, 2);
 octets[counter++] = b;
 }
 this.octets = octets;
 }
 public IPAddress and(IPAddress otherIP){
 int[] otherOctet = otherIP.getOctets();
 int[] thisOctets = this.getOctets();
 int[] newOctets = new int[4];
 for (int i = 0; i < otherOctet.length; i++) {
 newOctets[i] = thisOctets[i] & otherOctet[i];
 }
 return new IPAddress(newOctets);
 }
 public IPAddress or(IPAddress otherIP){
 int[] otherOctet = otherIP.getOctets();
 int[] thisOctets = this.getOctets();
 int[] newOctets = new int[4];
 for (int i = 0; i < otherOctet.length; i++) {
 newOctets[i] = otherOctet[i] | thisOctets[i];
 }
 return new IPAddress(newOctets);
 }
 public long toDecimal(){
 // turn each int into binary string
 String binString = "";
 for (int x : octets){
 //got to pad this:
 binString += padLeft(Integer.toBinaryString(x));
 }
 // turn it into a long:
 return Long.parseLong(binString,2);
 }
 @Override
 public String toString() {
 return StringUtils.implode(this.getOctets());
 }
}

StringUtils for manipulations that did not seem to belong in the other classes:

package com.company;
public class StringUtils {
 public static String padLeft (String input, int count, String pad){
 String output = "";
 for(int c = 0; c < count - input.length(); c++){
 output += pad;
 }
 return output + input;
 }
 public static String padLeft(String input){
 String pad = "0";
 return padLeft(input, 8, pad);
 }
 static String implode(int[] intArray){
 String result = "";
 for (int i = 0; i < intArray.length; i++) {
 int i1 = intArray[i];
 if (i != intArray.length -1 ) {
 result += i1 + ".";
 } else {
 result += i1;
 }
 }
 return result;
 }
}

Finally, The Unit Tests:

package com.company;
import org.junit.Test;
import java.util.ArrayList;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
public class CidrTest {
 @Test(expected=InvalidIPException.class)
 public void testInvalidLarge() throws Exception {
 IPAddress ipOne = new IPAddress("10.20.33.500");
 }
 @Test(expected=InvalidIPException.class)
 public void testInvalidLetter() throws Exception {
 IPAddress ipTwo = new IPAddress("a.b.c.d");
 }
 @Test(expected=InvalidIPException.class)
 public void testInvalidSmall() throws Exception {
 IPAddress ipThree = new IPAddress("-1.10.-3.-4");
 }
 @Test(expected=InvalidIPException.class)
 public void testInvalidFormat() throws Exception {
 IPAddress ipFour = new IPAddress("20|33|3|0");
 }
 @Test(expected=InvalidIPException.class)
 public void testInvalidCount() throws Exception {
 IPAddress ipFour = new IPAddress("20.33.3.33.55");
 }
 @Test
 public void testCompareIP() throws Exception{
 IPAddress ipOne = new IPAddress("10.20.33.44");
 IPAddress ipTwo = new IPAddress("10.20.33.45");
 assertTrue(ipOne.compareTo(ipTwo) < 0);
 }
 @Test
 public void testIsInRange() throws Exception {
 // an IP is in range if the ip is between hostMin and hostmax
 IPAddress ipThree = new IPAddress("10.20.33.28");
 Cidr cidr = new Cidr("10.20.33.4/24");
 assertTrue(cidr.isInRange(ipThree));
 // null address should never be in range.
 assertFalse(cidr.isInRange(null));
 }
 @Test
 public void testCountHosts() throws Exception {
 Cidr cdr = new Cidr("10.10.15.10/16");
 assertEquals(65534,cdr.countHosts());
 cdr = new Cidr("10.10.1.97/23");
 assertEquals(510,cdr.countHosts());
 }
 @Test
 public void testGetNetMask() throws Exception {
 IPAddress ip = new IPAddress("255.255.0.0");
 assertEquals(ip, new Cidr("10.10.15.10/16").getNetMask());
 }
 @Test
 public void testGetNetworkAddress() throws Exception {
 IPAddress ip = new IPAddress("10.10.0.0");
 assertEquals(ip, new Cidr("10.10.15.10/16").getNetworkAddress());
 IPAddress ip2 = new IPAddress("10.10.0.0");
 Cidr cdr = new Cidr("10.10.1.97/23");
 assertEquals(ip2,cdr.getNetworkAddress());
 IPAddress net3 = new IPAddress("192.168.0.0");
 cdr = new Cidr("192.168.0.3/25");
 assertEquals(net3,cdr.getNetworkAddress());
 IPAddress net4 = new IPAddress("172.16.5.0");
 cdr = new Cidr("172.16.5.34/26");
 assertEquals(net4,cdr.getNetworkAddress());
 }
 @Test
 public void testGetBroadcast() throws Exception {
 // broadcast is inverted netmask ANDED with our network address:
 IPAddress broadcast = new IPAddress("10.10.255.255");
 assertEquals(broadcast,new Cidr("10.10.15.10/16").getBroadcastAddress());
 broadcast = new IPAddress("172.16.5.63");
 assertEquals(broadcast,new Cidr("172.16.5.34/26").getBroadcastAddress());
 }
 @Test
 public void testGetAddress() throws Exception {
 Cidr cdr = new Cidr("10.10.15.10/16");
 IPAddress ip = new IPAddress("10.10.15.10");
 assertEquals(ip, cdr.getAddress());
 }
 @Test
 public void testGetHostMax() throws Exception {
 Cidr cdr = new Cidr("10.10.233.233/23");
 IPAddress min = new IPAddress("10.10.233.254");
 assertEquals(min,cdr.getHostMax());
 }
 @Test
 public void testGetHostMin() throws Exception {
 Cidr cdr = new Cidr("10.10.233.233/23");
 IPAddress min = new IPAddress("10.10.232.1");
 assertEquals(min,cdr.getHostMin());
 }
 @Test
 public void testIpList() throws Exception {
 ArrayList<IPAddress> testArrayList = new ArrayList<IPAddress>();
 testArrayList.add(new IPAddress("10.10.233.233"));
 testArrayList.add(new IPAddress("10.10.233.234"));
 Cidr cdr = new Cidr("10.10.233.233/30");
 assertEquals(cdr.ipList(), testArrayList);
 }
}

Answer 1

Working on (削除) String (削除ここまで) (削除) int[] (削除ここまで) int

You're working on String and int[]. I have absolutely no idea why. The cool thing about CIDR notation is that it consists of 4 "octets". The value range 0-255 is exactly that of an unsigned byte.

Funnily enough, \4ドル * 8 = 32\$ ... Interestingly that's exactly the size of an Integer. What you need to represent an IPv4 address is a single int. If you want to keep track of a subnet (which is a significantly different thing), you just need to add another int for the subnet mask.

Properly working with this you can then determine the first and last IP address in a subnet by the following rather easy process:

int lowestAddress = address & subnetMask;
int highestAddress = address | !subnetMask;

Unfortunately java is a little unfriendly if we look for unsigned primitive types, which means that trying to parse 255 into a byte results in an Exception.

As soon as you consider that working on a single integer is significantly faster (and more idiomatic), I think it becomes obvious you should consider reimplementing this by using byte-shifting, bitmasks and bitwise operators.

I strongly suggest you completely reimplement that class using just that representation.

Nitpicks:

• You're indenting by 2 spaces. It's usual to indent by 4 spaces in java. Then again you are consistent, soo ...
• You're using default visibility a lot. I don't like that, because I'd expect that to be explained by a comment. That visibility is not needed very often.