search

darsyn / ip

Immutable value object for IPv4 and IPv6 addresses, including helper methods and Doctrine support.
http://darsyn.github.io/ip/
MIT License
245 stars 21 forks source link

Expanded IPv6 Formatter #56

Closed gjuric closed 5 years ago

gjuric commented 5 years ago

This PR adds a new Formatter that outputs IPv6 addresses in their expanded form.

I didn't squash the commits intentionally so you can see the implementation where ConsistentFormatter::ntopVersion6() remains a private function.

zanbaldwin commented 5 years ago

Thanks for making the effort to make this pull request! ☺️

Can you explain the reasoning for this addition? The formatters are only used in the IPv6::getCompactedAddress() method and not the IPv6::getExpandedAddress() method meaning that the new formatter you propose will change the intended functionality of IPv6::getCompactedAddress().

I don't think this particular change is necessary, but you have brought up a good point of whether there are too many methods in the IpInterface implementations (getDotAddress(), getCompactedAddress(), getExpandedAddress(), and getProtocolAppropriateAddress()) that deal with formatting, and whether that responsibility should be moved to additional formatters.

What are your thoughts? As a user of the library, what would be most useful for you?

gjuric commented 5 years ago

I am working with the instance of a Multi IP address and need a way to output the IP address in an extended format (if it is a v6 IP) and didn't want to check the version before calling the appropriate method. But now that I think of it that would probably be a better solution.

Thank you for your input.