3
\$\begingroup\$

Given a string representing of an IP address, such as "127.43.23.59", return a 32 bit integer representation of this string. The 32 bit integer is separated into four bytes, where each byte is one of the numbers in the string, separated by dots. If you & 0xFF with the returned integer, it should return 59. If you & 0xFF00 with the returned integer, it should return 23, etc.

public class IP{
    public static void main(String[] args)
    {
        IP address = new IP();
        int x = address.IPAddress("127.43.29.56");
    }
    public int IPAddress(String str)
    {
        int byte1 = 0;
        int numbers[] = new int[4];
        int index = 0;
        int finalByte = 0;
        for(int i = 0; i < str.length(); i++)
        {
            if(str.charAt(i) != '.')
                byte1 = 10*byte1 + (str.charAt(i) - 48);
            else
            {
                numbers[index++] = byte1;
                byte1 = 0;
            }
        }
        numbers[index] = byte1;
        return finalByte = (numbers[0]<<24) | (numbers[1]<<16)|
                    (numbers[2]<<8) | (numbers[3]);
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ You surely mean that "If you & 0xFF00 with the returned integer, it should return 5888" \$\endgroup\$
    – rolfl
    Commented Mar 20, 2016 at 12:40

1 Answer 1

3
\$\begingroup\$

First impressions

Some basic issues that immediately jump in the eye:

  • A method that doesn't need data in member fields can be static. The IPAddress method looks like a good candidate for a static utility method in an IPAddressUtils class.
  • The Java convention is camelCase for method names (violated by IPAddress)
  • Poor naming: the IPAddress method takes a dotted decimal notation and returns a 32-bit integer representation. The name "IPAddress" does very little to describe that
  • finalByte is unused, unnecessary
  • Unusual declaration: instead of int numbers[], the convention is int[] numbers
  • Unusual formatting

Simplification

You don't need an int[] to calculate the value. You need just one value, and shift it to the left by 8 bits for each parsed byte value.

Instead of the number 48 for '0', you can just use '0' to make the meaning obvious.

Suggested implementation

Applying the above suggestions, here's a utility class:

public class IPAddressUtils {

    public static int toInt(String dottedDecimal) {
        int byte1 = 0;
        int value = 0;
        for (char c : dottedDecimal.toCharArray()) {
            if (c != '.') {
                byte1 = 10 * byte1 + (c - '0');
            } else {
                value <<= 8;
                value += byte1;
                byte1 = 0;
            }
        }
        value <<= 8;
        value += byte1;
        return value;
    }
}
\$\endgroup\$

Your Answer

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