14
\$\begingroup\$

The purpose of this little program is to:

  • Separate IPv4 addresses and IPv6 addresses
  • Will be able to sort them even if the formatting of the input is not great
  • Sort them into an order
  • Print them into the system terminal in ascending and descending order

I would like to know if you think the program what it is meant to do properly. I would particularly be interested in your thoughts on the efficiency of the program and if any improvements can be made to it.

import java.util.*;
import java.math.*;
import java.util.regex.*;
import java.lang.*;

public class IPSort
{
    String[] tests = {":8:","::::5:6::8","::::5:6::7","::::5:6::8","123..245.23","1...","..1.","123...23",".1..","123..245.23", "123..245.23", "104.244.253.29", "1.198.3.93", "32.183.93.40", "32.183.93.40", "104.30.244.2", "104.244.4.1","0.0.0.1",":a:","::5:3:4:5:6:78","1::2:3","1::2:3:4","1::5:256.2.3.4","1:1:3000.30.30.30","ae80::217:f2ff:254:7:237:98"};
    ArrayList<String> bit32 = new ArrayList<String>();
    ArrayList<String> bit128 = new ArrayList<String>();
    ArrayList<String> cleanBit32 = new ArrayList<String>();
    ArrayList<String> cleanBit128 = new ArrayList<String>();

    boolean myMatcher32Bit(String s)
    {
        Pattern patter32Bit = Pattern.compile("^(?=(?:[^.]*\\.){3}[^.]*$)(?=(?:[^:]*:){0}[^:]*$)(?=(?:[^a-zA-Z]*[^a-zA-Z])*$)");
        Matcher matcher32Bit = patter32Bit.matcher(s);
        return matcher32Bit.find();
    }

    boolean myMatcher128Bit(String s)
    {
        Pattern patter128Bit = Pattern.compile("^(?=(?:[^.]*\\.){0}[^.]*$)(?=(?:[^:]*:){1,7}[^:]*$)");
        Matcher matcher128Bit = patter128Bit.matcher(s);
        return matcher128Bit.find();
    }

    public void sortIntoRespectiveIPTypes()
    {
        for(String s: tests)
        {
            if(myMatcher32Bit(s))
            {
                bit32.add(s);
            }
            else if(myMatcher128Bit(s))
            {
                bit128.add(s);
            }
        }

        System.out.println("32 bit IPs");
        for(String ip: bit32)
        {
            System.out.println("  "+ip);
        }

        System.out.println("\n128 bit IPs");
        for(String ip: bit128)
        {
            System.out.println("  "+ip);
        }

        int count = 0;
        for(String ip: tests)
        {
            if(myMatcher32Bit(ip)==false && myMatcher128Bit(ip)==false)
            {
                count++;
            }
        }

        if(count != 0)
        {
            System.out.println("\nDidn't match an IP format");
            for(String ip: tests)
            {
                if(myMatcher32Bit(ip)==false && myMatcher128Bit(ip)==false)
                {
                    System.out.println("  "+ip);
                }
            }
        }

    }

    public void sort32BitIPs(ArrayList<String> bit32, ArrayList<String> newBit32)
    {
        ArrayList<BigInteger> bigInt32Bit = new ArrayList<BigInteger>();
        for(String ip:bit32)
        {
            String[] tempArray = ip.split("\\.");
            int i=0;
            for(String s:tempArray)
            {
                if(s.equals(""))
                {
                    tempArray[i]="0";
                }
                i++;
            }
            bigInt32Bit.add(convert32Bit(tempArray));
        }

        Collections.sort(bigInt32Bit);

        ArrayList<String> fixFormat = new ArrayList<String>();
        for(String ip:bit32)
        {
            String[] fixArray = ip.split("\\.");
            int i=0;
            for(String s:fixArray)
            {
                if(s.equals(""))
                {
                    fixArray[i]="0";
                }
                i++;
            }

            StringBuilder strBuilder = new StringBuilder();
            for(int i2 = 0; i2 < 4; i2++) 
            {
                if(i2<3)
                {
                    try
                    {
                        if(!fixArray[i2].equals(""))
                        {
                            strBuilder.append(fixArray[i2]+".");
                        }
                        else
                        {
                            strBuilder.append(".");
                        }
                    }
                    catch(Exception e)
                    {
                        strBuilder.append("0.");
                    }
                }
                else
                {
                    try
                    {
                        strBuilder.append(fixArray[i2]);
                    }
                    catch(Exception e)
                    {
                        strBuilder.append("0");
                    }
                }
            }
            String newString = strBuilder.toString();
            fixFormat.add(newString);
            bit32=fixFormat;
        }

        for(BigInteger finalValue:bigInt32Bit)
        {
            for(String ip:bit32)
            {
                String[] tempArray = ip.split("\\.");
                int i=0;
                for(String s:tempArray)
                {
                    if(s.equals(""))
                    {
                        tempArray[i]="0";
                    }
                    i++;
                }
                if(finalValue.equals(convert32Bit(tempArray)))
                {
                    if(!newBit32.contains(ip))
                    {
                        String str = bit32.toString();
                        String findStr = ip;
                        int lastIndex = 0;
                        int count = 0;

                        while(lastIndex != -1){

                            lastIndex = str.indexOf(findStr,lastIndex);

                            if(lastIndex != -1){
                                count++;
                                lastIndex += findStr.length();
                            }
                        }

                        for(int k = 0; k<count;k++)
                        {
                            newBit32.add(ip);
                        }
                    }
                }
            }
        }
    }

    BigInteger convert32Bit(String[] array)
    {
        int[] tempArray = new int[array.length];
        ArrayList<BigInteger> tempBigIntList = new ArrayList<BigInteger>();
        int i = 0;
        for(String s:array)
        {
            int power = 4-i;
            tempArray[i]= Integer.parseInt(s);
            String string = Integer.toString(tempArray[i]);
            BigInteger myBigInt = new BigInteger(string);
            BigInteger num2 = myBigInt.multiply(new BigInteger("256").pow(power));
            tempBigIntList.add(num2);
            i++;
        }
        BigInteger bigInt32Bit = new BigInteger("0");
        for(BigInteger bI:tempBigIntList)
        {
            bigInt32Bit = bigInt32Bit.add(bI);
        }
        return bigInt32Bit;
    }

    public void sort128BitIPs(ArrayList<String> bit128,ArrayList<String> newBit128)
    {
        ArrayList<BigInteger> bigInt128Bit = new ArrayList<BigInteger>();
        for(String ip:bit128)
        {
            String[] tempArray = ip.split(":");
            int i=0;
            for(String s:tempArray)
            {
                if(s.equals(""))
                {
                    tempArray[i]="0";
                }
                i++;
            }
            bigInt128Bit.add(convert128Bit(tempArray));
        }

        Collections.sort(bigInt128Bit);

        for(BigInteger finalValue:bigInt128Bit)
        {
            for(String ip:bit128)
            {
                String[] tempArray = ip.split(":");
                int i=0;
                for(String s:tempArray)
                {
                    if(s.equals(""))
                    {
                        tempArray[i]="0";
                    }
                    i++;
                }
                if(finalValue.equals(convert128Bit(tempArray)))
                {
                    if(!newBit128.contains(ip))
                    {
                        String str = bit128.toString();
                        String findStr = ip;
                        int lastIndex = 0;
                        int count = 0;

                        while(lastIndex != -1){

                            lastIndex = str.indexOf(findStr,lastIndex);

                            if(lastIndex != -1){
                                count++;
                                lastIndex += findStr.length();
                            }
                        }

                        for(int k = 0; k<count;k++)
                        {
                            newBit128.add(ip);
                        }
                    }
                }
            }
        }
    }

    BigInteger convert128Bit(String[] array)
    {
        int[] tempArray = new int[array.length];
        ArrayList<BigInteger> tempBigIntList = new ArrayList<BigInteger>();
        int i = 0;
        for(String s:array)
        {
            int power = 8-i;
            tempArray[i]= Integer.parseInt(s,16);
            String string = Integer.toString(tempArray[i]);
            BigInteger myBigInt = new BigInteger(string);
            BigInteger num2 = myBigInt.multiply(new BigInteger("65536").pow(power));
            tempBigIntList.add(num2);
            i++;
        }
        BigInteger bigInt128Bit = new BigInteger("0");
        for(BigInteger bI:tempBigIntList)
        {
            bigInt128Bit = bigInt128Bit.add(bI);
        }
        return bigInt128Bit;
    }

    public void printInOrder(ArrayList<String> bit32,ArrayList<String> bit128)
    {
        System.out.println("\nSorted IPs");

        System.out.println("Sorted 32 bit IPs - Ascending");
        for(String ip: bit32)
        {
            System.out.println("  "+ip);
        }

        Collections.reverse(bit32);
        System.out.println("\nSorted 32 bit IPs - Descending");
        for(String ip: bit32)
        {
            System.out.println("  "+ip);
        }

        System.out.println("\nSorted 128 bit IPs - Ascending");
        for(String ip: bit128)
        {
            System.out.println("  "+ip);
        }

        Collections.reverse(bit128);
        System.out.println("\nSorted 128 bit IPs - Descending");
        for(String ip: bit128)
        {
            System.out.println("  "+ip);
        }
    }

    public void run(ArrayList<String> bit32,ArrayList<String> bit128,ArrayList<String> newBit32,ArrayList<String> newBit128)
    {
        sortIntoRespectiveIPTypes();
        sort32BitIPs(bit32,newBit32);
        sort128BitIPs(bit128,newBit128);
        printInOrder(newBit32,newBit128);
    }

    public static void main(String[] args)
    {
        IPSort ipS = new IPSort();
        ipS.run(ipS.bit32,ipS.bit128,ipS.cleanBit32,ipS.cleanBit128);
    }
}

Output

Output

Any other comments, tips or suggestions are welcomed and appreciated.

Improve this question
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Have you considered treating IPv4 addresses as IPv4-mapped addresses and sorting all of the addresses in the same IPv6 address space? \$\endgroup\$
    – 200_success
    Dec 23, 2015 at 22:55
  • 2
    \$\begingroup\$ Many of the 128-bit IP addresses in your test are actually invalid according to RFC 4291 Sec 2.2: "The preferred form is x:x:x:x:x:x:x:x, where the 'x's are one to four hexadecimal digits" — not zero to four. Zeroes can be compressed, but only using a double colon. \$\endgroup\$
    – 200_success
    Dec 23, 2015 at 23:04
  • \$\begingroup\$ @200_success Thank you for the information I shall look into these things \$\endgroup\$
    – Dan
    Dec 24, 2015 at 0:01

2 Answers 2

Reset to default
4
\$\begingroup\$

You are reinventing the wheel, and doing it incorrectly.

Java already has a java.net.InetAddress class to represent IP addresses, with subclasses Inet4Address and Inet6Address. You can parse addresses using InetAddress.getByName(…).

Your IPv6 parsing is wrong. Don't assume that IPv6 addresses behave like IPv4 addresses with colons instead of dots and hexadecimal instead of decimal numbers; read RFC2765 Sec 2.1 for the specification. In particular, most of the addresses you had in your example are actually invalid IPv6 addresses. A double-colon in an IPv6 address expands to as many zeros as necessary to fill the 128 bits; there can be at most one double-colon in an address.

Once you have a class to represent IP addresses (whether using the standard InetAddress or one of your own invention), a good way to sort them is to define a Comparator. It's much cleaner to sort objects using the right comparator than to try to parse strings into a BigInteger representation, sort the addresses as numbers, and reformat the results as strings.

In the demonstration below, I have used Java 8 streams for convenience. The result is substantially shorter than your original code, and more standards-compliant.

import java.net.InetAddress;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.Optional;
import java.util.stream.Stream;

public class IPSort {
    private static String[] TESTS = { … };

    /**
     * Comparator for InetAddresses.  IPv4 are compared to IPv6
     * addresses by converting IPv4 addresses to IPv4-compatible
     * IPv6 addresses according to RFC2765 Sec 2.1.
     */
    public static class InetAddressComparator implements Comparator<InetAddress> {
        @Override
        public int compare(InetAddress a, InetAddress b) {
            byte[] aOctets = a.getAddress(),
                   bOctets = b.getAddress();
            int len = Math.max(aOctets.length, bOctets.length);
            for (int i = 0; i < len; i++) {
                byte aOctet = (i >= len - aOctets.length) ?
                    aOctets[i - (len - aOctets.length)] : 0;
                byte bOctet = (i >= len - bOctets.length) ?
                    bOctets[i - (len - bOctets.length)] : 0;
                // Unsigned byte comparison (http://stackoverflow.com/a/19186)
                if (aOctet != bOctet) return (0xff & aOctet) - (0xff & bOctet);
            }
            return 0;
        }
    }

    public static Optional<InetAddress> toInetAddress(String s) {
        try {
            return Optional.of(InetAddress.getByName(s));
        } catch (UnknownHostException badAddress) {
            return Optional.empty();
        }
    }

    public static void main(String[] args) throws Exception {
        System.out.println("Valid 32-bit addresses");
        Arrays.stream(TESTS)
              .map(IPSort::toInetAddress)
              .filter(Optional::isPresent)
              .map(Optional::get)
              .filter((addr) -> addr instanceof Inet4Address)
              .map(InetAddress::getHostAddress)
              .forEach(System.out::println);

        System.out.println("\nValid 128-bit addresses");
        Arrays.stream(TESTS)
              .map(IPSort::toInetAddress)
              .filter(Optional::isPresent)
              .map(Optional::get)
              .filter((addr) -> addr instanceof Inet6Address)
              .map(InetAddress::getHostAddress)
              .forEach(System.out::println);

        System.out.println("\nInvalid addresses");
        Arrays.stream(TESTS)
              .filter((s) -> !toInetAddress(s).isPresent())
              .forEach(System.out::println);

        System.out.println("\nSorted addresses");
        Arrays.stream(TESTS)
              .map(IPSort::toInetAddress)
              .filter(Optional::isPresent)
              .map(Optional::get)
              .sorted(new InetAddressComparator())
              .map(InetAddress::getHostAddress)
              .forEach(System.out::println);

        System.out.println("\nReverse-sorted 32-bit addresses");
        Arrays.stream(TESTS)
              .map(IPSort::toInetAddress)
              .filter(Optional::isPresent)
              .map(Optional::get)
              .filter((addr) -> addr instanceof Inet4Address)
              .sorted(Collections.reverseOrder(new InetAddressComparator()))
              .map(InetAddress::getHostAddress)
              .forEach(System.out::println);
    }

}
Improve this answer
\$\endgroup\$
5
\$\begingroup\$

Import Statements

Before I start complaining about your import statements, I want you to know that the package declaration import java.lang.* is implied in all Java programs and is not necessary.

Never import the whole package. Always import only what you need. So in the case of your import statements:

import java.util.*;
import java.math.*;
import java.util.regex.*;
import java.lang.*;

It would be:

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collections;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Looks a lot more organized, and you see exactly what you need.

Formatting

I see some formatting issues in your code. In method declarations:

public void sort128BitIPs(ArrayList<String> bit128,ArrayList<String> newBit128)

Parameters should be separated with a comma followed by a space, not just a comma. You do this once, but just once.

Here:

        tempArray[i]= Integer.parseInt(s);

There should be always spaces before and after an operator, not just =. In your code, you also have the same problem with +.

About your bracing style, you mostly have the braces on a new line, but here:

                    while(lastIndex != -1){

                        lastIndex = str.indexOf(findStr,lastIndex);

                        if(lastIndex != -1){
                            count++;
                            lastIndex += findStr.length();
                        }
                    }

You don't.

Choose a style, and stick to it. I suggest the brace on the same line, as that is what the standard Java conventions recommend, but it's your choice.

List declarations

Instead of declaring an ArrayList as an ArrayList, it is better practice to declare it to just List. Here:

ArrayList<String> bit32 = new ArrayList<String>();
ArrayList<String> bit128 = new ArrayList<String>();
ArrayList<String> cleanBit32 = new ArrayList<String>();
ArrayList<String> cleanBit128 = new ArrayList<String>();

Should be:

List<String> bit32 = new ArrayList<String>();
List<String> bit128 = new ArrayList<String>();
List<String> cleanBit32 = new ArrayList<String>();
List<String> cleanBit128 = new ArrayList<String>();

And this:

public void run(ArrayList<String> bit32,ArrayList<String> bit128,ArrayList<String> newBit32,ArrayList<String> newBit128)
{
    sortIntoRespectiveIPTypes();
    sort32BitIPs(bit32,newBit32);
    sort128BitIPs(bit128,newBit128);
    printInOrder(newBit32,newBit128);
}

Turns into:

public void run(List<String> bit32, List<String> bit128, List<String> newBit32, List<String> newBit128)
{
    sortIntoRespectiveIPTypes();
    sort32BitIPs(bit32,newBit32);
    sort128BitIPs(bit128,newBit128);
    printInOrder(newBit32,newBit128);
}

And everywhere else you declare them as ArrayLists, change it into List.

The code so far:

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class IPSort {

    String[] tests = { ":8:", "::::5:6::8", "::::5:6::7", "::::5:6::8",
            "123..245.23", "1...", "..1.", "123...23", ".1..", "123..245.23",
            "123..245.23", "104.244.253.29", "1.198.3.93", "32.183.93.40",
            "32.183.93.40", "104.30.244.2", "104.244.4.1", "0.0.0.1", ":a:",
            "::5:3:4:5:6:78", "1::2:3", "1::2:3:4", "1::5:256.2.3.4",
            "1:1:3000.30.30.30", "ae80::217:f2ff:254:7:237:98" };
    List<String> bit32 = new ArrayList<String>();
    List<String> bit128 = new ArrayList<String>();
    List<String> cleanBit32 = new ArrayList<String>();
    List<String> cleanBit128 = new ArrayList<String>();

    boolean myMatcher32Bit(String s) {
        Pattern patter32Bit = Pattern.compile(
                "^(?=(?:[^.]*\\.){3}[^.]*$)(?=(?:[^:]*:){0}[^:]*$)(?=(?:[^a-zA-Z]*[^a-zA-Z])*$)");
        Matcher matcher32Bit = patter32Bit.matcher(s);
        return matcher32Bit.find();
    }

    boolean myMatcher128Bit(String s) {
        Pattern patter128Bit = Pattern.compile(
                "^(?=(?:[^.]*\\.){0}[^.]*$)(?=(?:[^:]*:){1,7}[^:]*$)");
        Matcher matcher128Bit = patter128Bit.matcher(s);
        return matcher128Bit.find();
    }

    public void sortIntoRespectiveIPTypes() {
        for (String s : tests) {
            if (myMatcher32Bit(s)) {
                bit32.add(s);
            } else if (myMatcher128Bit(s)) {
                bit128.add(s);
            }
        }

        System.out.println("32 bit IPs");
        for (String ip : bit32) {
            System.out.println("  " + ip);
        }

        System.out.println("\n128 bit IPs");
        for (String ip : bit128) {
            System.out.println("  " + ip);
        }

        int count = 0;
        for (String ip : tests) {
            if (myMatcher32Bit(ip) == false && myMatcher128Bit(ip) == false) {
                count++;
            }
        }

        if (count != 0) {
            System.out.println("\nDidn't match an IP format");
            for (String ip : tests) {
                if (myMatcher32Bit(ip) == false && myMatcher128Bit(
                        ip) == false) {
                    System.out.println("  " + ip);
                }
            }
        }

    }

    public void sort32BitIPs(List<String> bit32, List<String> newBit32) {
        List<BigInteger> bigInt32Bit = new ArrayList<BigInteger>();
        for (String ip : bit32) {
            String[] tempArray = ip.split("\\.");
            int i = 0;
            for (String s : tempArray) {
                if (s.equals("")) {
                    tempArray[i] = "0";
                }
                i++;
            }
            bigInt32Bit.add(convert32Bit(tempArray));
        }

        Collections.sort(bigInt32Bit);

        List<String> fixFormat = new ArrayList<String>();
        for (String ip : bit32) {
            String[] fixArray = ip.split("\\.");
            int i = 0;
            for (String s : fixArray) {
                if (s.equals("")) {
                    fixArray[i] = "0";
                }
                i++;
            }

            StringBuilder strBuilder = new StringBuilder();
            for (int i2 = 0; i2 < 4; i2++) {
                if (i2 < 3) {
                    try {
                        if (!fixArray[i2].equals("")) {
                            strBuilder.append(fixArray[i2] + ".");
                        } else {
                            strBuilder.append(".");
                        }
                    } catch (Exception e) {
                        strBuilder.append("0.");
                    }
                } else {
                    try {
                        strBuilder.append(fixArray[i2]);
                    } catch (Exception e) {
                        strBuilder.append("0");
                    }
                }
            }
            String newString = strBuilder.toString();
            fixFormat.add(newString);
            bit32 = fixFormat;
        }

        for (BigInteger finalValue : bigInt32Bit) {
            for (String ip : bit32) {
                String[] tempArray = ip.split("\\.");
                int i = 0;
                for (String s : tempArray) {
                    if (s.equals("")) {
                        tempArray[i] = "0";
                    }
                    i++;
                }
                if (finalValue.equals(convert32Bit(tempArray))) {
                    if (!newBit32.contains(ip)) {
                        String str = bit32.toString();
                        String findStr = ip;
                        int lastIndex = 0;
                        int count = 0;

                        while (lastIndex != -1) {

                            lastIndex = str.indexOf(findStr, lastIndex);

                            if (lastIndex != -1) {
                                count++;
                                lastIndex += findStr.length();
                            }
                        }

                        for (int k = 0; k < count; k++) {
                            newBit32.add(ip);
                        }
                    }
                }
            }
        }
    }

    BigInteger convert32Bit(String[] array) {
        int[] tempArray = new int[array.length];
        List<BigInteger> tempBigIntList = new ArrayList<BigInteger>();
        int i = 0;
        for (String s : array) {
            int power = 4 - i;
            tempArray[i] = Integer.parseInt(s);
            String string = Integer.toString(tempArray[i]);
            BigInteger myBigInt = new BigInteger(string);
            BigInteger num2 = myBigInt.multiply(new BigInteger("256").pow(
                    power));
            tempBigIntList.add(num2);
            i++;
        }
        BigInteger bigInt32Bit = new BigInteger("0");
        for (BigInteger bI : tempBigIntList) {
            bigInt32Bit = bigInt32Bit.add(bI);
        }
        return bigInt32Bit;
    }

    public void sort128BitIPs(List<String> bit128, List<String> newBit128) {
        List<BigInteger> bigInt128Bit = new ArrayList<BigInteger>();
        for (String ip : bit128) {
            String[] tempArray = ip.split(":");
            int i = 0;
            for (String s : tempArray) {
                if (s.equals("")) {
                    tempArray[i] = "0";
                }
                i++;
            }
            bigInt128Bit.add(convert128Bit(tempArray));
        }

        Collections.sort(bigInt128Bit);

        for (BigInteger finalValue : bigInt128Bit) {
            for (String ip : bit128) {
                String[] tempArray = ip.split(":");
                int i = 0;
                for (String s : tempArray) {
                    if (s.equals("")) {
                        tempArray[i] = "0";
                    }
                    i++;
                }
                if (finalValue.equals(convert128Bit(tempArray))) {
                    if (!newBit128.contains(ip)) {
                        String str = bit128.toString();
                        String findStr = ip;
                        int lastIndex = 0;
                        int count = 0;

                        while (lastIndex != -1) {

                            lastIndex = str.indexOf(findStr, lastIndex);

                            if (lastIndex != -1) {
                                count++;
                                lastIndex += findStr.length();
                            }
                        }

                        for (int k = 0; k < count; k++) {
                            newBit128.add(ip);
                        }
                    }
                }
            }
        }
    }

    BigInteger convert128Bit(String[] array) {
        int[] tempArray = new int[array.length];
        List<BigInteger> tempBigIntList = new ArrayList<BigInteger>();
        int i = 0;
        for (String s : array) {
            int power = 8 - i;
            tempArray[i] = Integer.parseInt(s, 16);
            String string = Integer.toString(tempArray[i]);
            BigInteger myBigInt = new BigInteger(string);
            BigInteger num2 = myBigInt.multiply(new BigInteger("65536").pow(
                    power));
            tempBigIntList.add(num2);
            i++;
        }
        BigInteger bigInt128Bit = new BigInteger("0");
        for (BigInteger bI : tempBigIntList) {
            bigInt128Bit = bigInt128Bit.add(bI);
        }
        return bigInt128Bit;
    }

    public void printInOrder(List<String> bit32, List<String> bit128) {
        System.out.println("\nSorted IPs");

        System.out.println("Sorted 32 bit IPs - Ascending");
        for (String ip : bit32) {
            System.out.println("  " + ip);
        }

        Collections.reverse(bit32);
        System.out.println("\nSorted 32 bit IPs - Descending");
        for (String ip : bit32) {
            System.out.println("  " + ip);
        }

        System.out.println("\nSorted 128 bit IPs - Ascending");
        for (String ip : bit128) {
            System.out.println("  " + ip);
        }

        Collections.reverse(bit128);
        System.out.println("\nSorted 128 bit IPs - Descending");
        for (String ip : bit128) {
            System.out.println("  " + ip);
        }
    }

    public void run(List<String> bit32, List<String> bit128,
            List<String> newBit32, List<String> newBit128) {
        sortIntoRespectiveIPTypes();
        sort32BitIPs(bit32, newBit32);
        sort128BitIPs(bit128, newBit128);
        printInOrder(newBit32, newBit128);
    }

    public static void main(String[] args) {
        IPSort ipS = new IPSort();
        ipS.run(ipS.bit32, ipS.bit128, ipS.cleanBit32, ipS.cleanBit128);
    }

}

OOPness

You have a lot of default level variables and methods. Instead, make them private.

Your code is not very OOP. Yes, I don't see static methods, but you are including the test parameters inside the class, which is just as bad. You also print in the class, which should be done outside. This all violates SRP (Single Responsibility Principle).

I suggest have a separate class for just sorting, and just testing. Think about:

  • Is this class only doing what it is supposed to do?
    • Good answer: Yes!
    • Bad answer: Not really...
  • Is this class doing too many things?
    • Good answer: No!
    • Bad answer: Yep...
  • Is this class well-structured (i.e. inheritance-wise and method-wise)?
    • Good answer: Yes!
    • Bad answer: Not really...

Since that would take a while, I'll leave it up to you...

Improve this answer
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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