IPv4 struct: Round 2

Summary

Original Question here.
Updated GitHub link here

I've taken an interest in lower level data structures and I implemented an 'Ip4Address to get familiar with Explicit structure layout. It turned out to be an exercise in properly implementing a struct in general, but I digress. The idea here is that each Octet of the IPv4 address is represented by a byte in a single 32 bit unsigned integer.

Things I've addressed since last review:

• Better argument validation all around.

• The struct is now immutable.

• Extracted a method for validating Ip addresses passed as strings.

• The code no longer needlessly initializes the struct only to overwrite it's values.

• Implemented IEquatable & IComparable.






• In addition, I implemented and overrode the other methods that are recommended when implementing those two interfaces. This includes Object.Equals(Object), GetHashCode(), and the equality operators.

• Added missing test cases.

• Implemented a ToByteArray() method.

• Removed useless variables from test cases.

• Use a method group for address.Split('.').Select(a => Byte.Parse(a)).ToArray()

• I'm still on the fence about this last one. I like the explicitness of the syntax I originally used, but I changed it anyway.

Things I did not address:

• Using Assert.Throws() for validating exceptions in my tests.

• I'm not switching to NUnit. I like MSTest.

• Expression bodied member for ToString() override.

• I'm hanging onto my curly braces for now. Expression bodied members are still feeling really weird to me.

• Properties for the Octets.

• It felt really weird to explicitly lay out the structure of the struct, then to hide it all behind properties. I'm open to hearing a solid argument for doing this though. I just wasn't convinced during my last review.

• Use uint instead of UInt32






• I explicitly used UInt32 to make it clear that Address` is a 32 bit integer and that the Octets use the same memory space.

Questions:

• Did I get

Answering your questions.

-
Your GetHashCode() implementation is good enough.

-
It's better to store octets in the reverse order.

The highest bits of an address should be stored in the Octet4, the lowest - in the Octet1.

Thus you will be able to simplify your CompareTo method:

public int CompareTo(Ip4Address other)
{
    return Address.CompareTo(other.Address);
}

-
And yes, this code depends on the endianness. If you are planning to use it on Big Endian systems too, you could use the BitConverter.IsLittleEndian static field to detect endianness and reverse octets if needed:

var bytes = address.Split('.').Select(Byte.Parse);
if (BitConverter.IsLittleEndian)
{
    bytes = bytes.Reverse();
}
return bytes.ToArray();

In this case you should also add a remark to descriptions of the Ip4Address(byte[] address) constructor and to the ToByteArray method, that the byte order depends on the endianness.

-
Your Equals(object obj) method can be simplified to:

return obj is Ip4Address && this.Equals((Ip4Address)obj);

My vision of the ParseStringAddress static method:

private static byte[] ParseStringAddress(string address)
{
    if (address == null)
    {
        throw new ArgumentNullException(nameof(address));
    }

    string[] inputOctets = address.Split('.');

    const int expectedMaxLength = sizeof(uint);

    // Questionable.
    // Should we handle short forms of the IPv4, such as
    //   10.1.1
    //   192.168
    //   0
    // or not?
    // If not, rename the 'expectedMaxLength' back to the 'expectedLength'
    // and replace the '>' with the '!=' in the following condition:
    if (inputOctets.Length > expectedMaxLength)
    {
        throw new ArgumentException(...);
    }

    byte[] outputOctets = new byte[expectedMaxLength];
    for (int i = 0; i < inputOctets.Length; i++)
    {
        int outputOctetIndex = BitConverter.IsLittleEndian ? expectedMaxLength - 1 - i : i;
        if (!Byte.TryParse(inputOctets[i], out outputOctets[outputOctetIndex]))
        {
            throw new ArgumentException(...);
        }
    }

    return outputOctets;
}

In addition to the normal input strings it also accepts input strings:

• Questionable. With the short form of IPv4 address representing a subnet, such as 10.1.1 (equivalent of 10.1.1.0).

• Containing space chars, such as 1 . 2 . 3 . 4.

This method outputs octets in the right order depending on the BitConverter.IsLittleEndian value.

StackExchange Code Review Q#112149, answer score: 4