search

jsakamoto / ipaddressrange

.NET Class Library for range of IP address, both IPv4 and IPv6.
Mozilla Public License 2.0
370 stars 71 forks source link

TryParse throws exception if null is passed #14

Closed gregmac closed 9 years ago

gregmac commented 9 years ago

I don't think TryParse should be throwing an exception for any type of input. It's currently limited to FormatException and I'm not sure why to not just catch any Exception and return false.

jsakamoto commented 9 years ago

I'm not sure why to not just catch any Exception and return false.

because, I'm afraid that I can't catch what bug in these codes.

If we catch all types of exception without thought, we could not know what's happen.

I had feeled that situation was foolish...

But, exactly as you say, perhaps we should catch any exceptions to keep more safety.

Please let me few days to cosider well about your pull request.

gregmac commented 9 years ago

That's fair, though I'd say that there should be enough unit tests to be confident in knowing there are not any situations that throw an unexpected exception.

Either way, at the very least, this is a valid test case that should be added and the TryParse code should at least catch the ArgumentNullException.

For reference, bool.TryParse handles things explicitly, and several other TryParse methods do the same.

IPAddress.TryParse calls an internal TryParse method that is a heap of ugliness, but does lots of this:

  if (tryParse) {
    return null;
  }
  throw new ArgumentNullException("ipString");
M-Zuber commented 9 years ago

I agree with @gregmac (see #7). In fact even though I made the PR for TryParse, I don't use it due to this issue.