What if you have NONE for return

1

I have coded myself into a corner. Not the first time. The below code rarely returns None. But if it does, it of course panics. If I try to handle the None case for RE.captures(&text), I get an error that I should be returning type IpAddr. How can I handle the option type from RE.

fn address_helper(text: &str) -> IpAddr {

    lazy_static! {
        static ref RE: Regex = Regex::new(r"\d+\.\d+\.\d+\.\d+").unwrap();
    }
    let caps = RE.captures(&text).unwrap();
    return IpAddr::from_str(caps.get(0)
        .map_or("",|m| m.as_str())).unwrap()
}
2

std::net::IpAddr implements FromStr, which you might prefer over rolling out your own parsing logic. Note how IpAddr::from_str returns a Result<IpAddr, AddrParseError>. Using Result as the return type is a great choice for a fallible parsing operation in my opinion.

1 Like
3

Without panicking? Change the signature and don't unwrap.

fn address_helper(text: &str) -> Result<IpAddr, AddrParseError> {
    lazy_static! {
        static ref RE: Regex = Regex::new(r"\d+\.\d+\.\d+\.\d+").unwrap();
    }
    let caps = RE.captures(&text).unwrap();
    IpAddr::from_str(
        caps.get(0)
            .map_or("",|m| m.as_str())
    )
}

The error may be misleading when the regex didn't match anything. If you want to handle that, you're probably entering custom or erased error territory (thiserror and anyhow are two popular crates related to that).

But I'm not sure how much sense that makes for this sort of inherently sloppy use case.

If you want to forego errors altogether and return Option<_>, you can call .ok() on the Result. This will make the non-match case less special, but provide no information about why inputs like 1.2.3.400 didn't return an address.

Alternatively you could return some default address (but personally I don't know that I'd call that "helping" the caller anymore).

They're already using FromStr; the Regex is to pull out some . separated digits that may be an IPv4. The FromStr implementation doesn't go searching for the first "maybe IPv4" in arbitrary text.

2 Likes
4

A couple of things. First, I am using the Regex against this, of course the IP address could be anything

"IP ADDRESS: 0.0.0.0"

Second, why is it a sloppy use case? And thanks for the suggestions of returning the Option or using a Result.

5

Because it also accepts any of these:

IpUdrsisss: 1.2.3.4

Gateway: 1.2.3.255

NetworkMask: 1.2.3.0/24

ProxyExceptions: 1.2.3.4, 1.2.3.5, 1.2.3.88

Double, double toil and trouble;
Fire burn and caldron bubble.
Fillet of a fenny snake,
In the caldron b0.31.33.1l and bake;
Eye of newt and toe of frog,
Wool of bat and tongue of dog,
Adder's fork and blind-worm's sting,
Lizard's leg and howlet's wing,
...

Given that and the name, I assumed it was intentional, like "pull something that looks like an address out of this arbitrary text".

5 Likes
6

I should have been more explicit. It's the value returned from an SNMP variable binding. Of course, I assumed the manufacturer would always return a correct IPv4 format.

7

you'd be surprised what some telco operators or management devices/frameworks can do...
trust me, i know... :hair-tearing-smiley:

1 Like
8

We should take that discussion offline, lest I leave a trail for my employer.

9

Baking assumptions like that into panics (eg expect() instead of letting the weirdness propagate is still useful even if the assumption is correct, because then you can say not just "surely they're sending just a valid IP?" but "ok, we know they're sending just a valid IP" and look elsewhere for the issue. And if it's not correct, you have exactly where and when, and for Result, what!

Of course, as mentioned, it's quite common that sensible assumptions end up being wrong, and now you have to figure out what the heck is happening, which is always the real fun with these black box interop issues...

10

of course, i mean no explicit parties and intended it as enlightened pun... :slight_smile:

generally speaking, sometime standards implementations can surprise person.