Check two values for equality ignoring their direction

help
1

Hi,

I have a callback that gets called whenever two ports are connected/disconnected. I need to check if one of the ports is the port specified by my app (my_port) and turn that information into an event. It doesn't matter if my_port is port_a or port_b.

First I have done something like:

   if port_a == my_port {
       let event = if are_connected {
           Event::OutPortConnected(port_b.to_string())
       } else {
           Event::OutPortDisconnected(port_b.to_string())
       };
       
       Some(event)
   } else if port_b == my_port {
       let event = if are_connected {
           Event::OutPortConnected(port_a.to_string())
       } else {
           Event::OutPortDisconnected(port_a.to_string())
       };
       
       Some(event)
   } else {
       None
   }

Then I came up with this:

  let ports = [port_a, port_b];
    let (src, dest): (Vec<&str>, Vec<&str>) =
        ports.into_iter().partition(|p| *p == my_port);

    if let (Some(_src), Some(dest)) = (src.first(), dest.first()) {
        let event = match are_connected {
            true => Event::OutPortConnected(dest.to_string()),
            false => Event::OutPortDisconnected(dest.to_string()),
        };
        
        Some(event)
    } else {
        None
    }

I think the first one is easier to understand, but could get confusing because of the boilerplate - I have do do this for an input port, too.
The second one is not bad, but I don't like the Vecs.
I think there might be a better way to accomplish this.

edit

2

What about this?

use itertools::Itertools;

fn event_from_ports_3(
    my_port: &str,
    port_a: &str,
    port_b: &str,
    are_connected: bool,
) -> Option<Event> {
    [port_a, port_b]
        .into_iter()
        .permutations(2)
        .filter_map(|ports| {
            if ports[0] == my_port {
                Some(ports[1])
            } else {
                None
            }
        })
        .next()
        .map(|port| match are_connected {
            true => Event::OutPortConnected(port.to_string()),
            false => Event::OutPortDisconnected(port.to_string()),
        })
}
3

If there will only be two ports you can cheat by XORing the array index of the matching port with 1, which gives 0 for 1 and 1 for 0:

fn event_from_ports_3(
    my_port: &str,
    port_a: &str,
    port_b: &str,
    are_connected: bool,
) -> Option<Event> {
    let ports = [port_a, port_b];
    let i = ports.iter().position(|&port| port == my_port)?;
    let ctor = if are_connected {Event::OutPortConnected} else {Event::OutPortDisconnected};
    Some(ctor(ports[i ^ 1].to_string()))
}
4

Wow, itertools, xor-ing array indices... That's tuff, IMHO... :laughing:

As always: Divide and conquer:

Put repeating parts, things, or different responsibilities into its own functions or even closures, or group them separately etc.:

fn event_from_ports(
    my_port: &str,
    port_a: &str,
    port_b: &str,
    are_connected: bool
) -> Option<Event> { 
    let event = |port: &str| {
        if are_connected {
           Event::OutPortConnected(port.to_string())
        } else {
           Event::OutPortDisconnected(port.to_string())
        }
    };

    if port_a == my_port {
        Some(event(port_b))
    } else if port_b == my_port {
        Some(event(port_a))
    } else {
        None
    }
}

If you don't like closures, use a small helper function:

fn make_event(port: &str, connected: bool) -> Event {
    ...
}

Composition is king!

3 Likes
5

Consider a full match:

match (port_a, port_b, are_connected) {
  (my_port, port_b, true)  => Some(Event::OutPortConnected(port_b.to_string())),
  (my_port, port_b, false) => Some(Event::OutPortDisconnected(port_b.to_string())),
  (port_a, my_port, true)  => Some(Event::OutPortConnected(port_a.to_string())),
  (port_a, my_port, false) => Some(Event::OutPortDisconnected(port_a.to_string())),
  _ => None,
}

Or consider breaking up the checks a bit by identifying which port is the output port first:

let out_port = match (port_a, port_b) {
  (my_port, port_b) => Some(port_b),
  (port_a, my_port) => Some(port_a),
  _ => None,
}

if let Some(out_port) = out_port {
  if are_connected {
    Some(OutPortConnected(out_port.to_string()))
  } else {
    Some(OutPortDisconnected(out_port.to_string()))
  }
} else {
  None
}

There isn't a singular "right" way to do this, though; do what reads best, and change it if your needs change later.

6

It's unfortunate that my_port in the pattern gets bound as a new variable shadowing the outer one. I was going to reply that the last example was my favorite, but found it had an unreachable match arm.

The updated version still looks OK though:

I like when a match can show you the 2 cases and their 2 outcomes on 2 easy to parse lines.

2 Likes
7

The hint to "divide and conquer" is valid.
Maybe a helper struct with a more oo-like approach is easier to maintain:

8

Thank you all for your suggestions, the solution from @derspiny and @danjl1100 worked best for me. I like that it preserves the natural order of first fetching the port and then producing the event.

1 Like
9

You can simplify that a bit with an early return

1 Like