Need help with validating some variables in a functional way

// this is defined in a trait
fn new(k: usize, basis: Vec<Product<B, BaseSign>>, coeffs: Vec<f32>) -> Result<Self> {
    if k == 0 {
        bail!("k cannot equal 0!")
    }
    // if any prod doesnt have k elements error
    // if any two are the same error
}

A Product is simply:

struct Product<B, BaseSign>(Vec<B>, PhantomData<BaseSign>)
where
    B: BasisVector,
    BaseSign: Sign;

All that matters about it is that it is a Vec<B>.
I am also using anyhow.

There are two requirements for the thing I am returning:

  • All elements in basis must be unique.
  • All elements in basis must have k elements where k is a given value

It is decently easy to implement something like this with a for loop.
However, I want to do it functional-like. How would I do this in rust because every time I try I end up with a monstrosity because you can't return values from a closure.

Previous Attempt :nauseated_face: :nauseated_face: (Doesn't work):

Summary
let result = basis
            .iter()
            .enumerate()
            .map(|(i, prod)| {
                Ok(if prod.0.len() != k {
                    Left(bail!("\n{prod:#?}\nmust be size: {k:?}!\n"))
                } else if basis[i + 1..].contains(prod) {
                    Left(bail!(
                        "Basis must not contain two of the same element:\n{:?}",
                        prod
                    ))
                } else {
                    Right(())
                })
            })
            .fold(
		Right(()),
                |acc: Either<Result<Self>, ()>, next: Either<Result<Self>, ()>| match acc {
                    Left(err) => acc,
                    Right(unit) => match next {
                        Left(err) => Left(err),
                        Right(_) => Right(()),
                    },
                },
            );
	let phantom = PhantomData;
	match result {
	    Left(err) => err,
	    Right(_) => Ok( Self { k, basis, coeffs, phantom })
	}

My immediate thought process says

It's easy just iter then map then fold!

But I don't think it's as easy as that.

If you can simplify your code with a for loop, then do it so. Don't use a functional approach just because you think it's what the cool kids do.

On the other hand, if you want help from the community, put some effort on clarifying exactly what is exactly what you want to do. I'm having a hard time reading your post.

3 Likes

Oops sorry about the clarity (or lack thereof). I hope the post is clearer after the edit, Thank you.

Your fold looks like the existing implementation of collect for "collecting" into a Result<(), E> (also this impl is involved), except the latter has short circuiting.

The map and fold together look like an indirect way of doing try_for_each. Still, an ordinary for loop is usually even cleaner to write than try_for_each, at least you're planning to early-return on error from the containing function anyways.

The way you test for uniqueness does have some unnecessary-O(n²) vibes, so if it isn't the case that the basis is always relatively short anyways, doing something more efficient might be reasonable.

4 Likes

Thank you,
I suppose I should just use a for loop.

Out of curiosity what faster way is there to check? Hashmap? I tried this implementation but it was kind of a pain since I had to implement the Eq trait manually since I need custom behaviour. Also for my use case, I'm trying to conserve memory (not sure if that matters at this scale).

You can derive trait implementations such as PartialEq, Eq and Hash. Why did you need a custom implementation for your use case?

At what scale exactly? Memory consumption in Rust programs is really low. Drastically lower if you compare it with garbage-collected languages.

Don't prematurely optimize unless you have already measured your code against a given metric and you aren't happy with the results.

2 Likes

Yeah I suppose you're right no need to think about that now. Ill just continue and make this a hash set if it this becomes a bottleneck

Asymptomatically faster ways would usually involve either hashing or sorting (so an additional trait bound on Hash or Ord).

2 Likes

Sorry order matters in my data structure, unless i add a variable and tweak the Ord implementation, but that sounds like it would be more complex. So for now its KISS for me.

A sorting-based approach does not necessarily involve sorting the original Vec. For example, you could also use something like an additional Vec<&Product>, sort that one, and then check for (subsequent) duplicates in there.

I'm not saying that's what you should do - the simplest reasonable approach is sufficient, I'm just exploring what the options are and that Ord-based solutions exist (another one would e. g. be to involve a BTreeSet.)

2 Likes

Very clever. I'll look into this should I have performance issues to do with checking for duplicates. Thank you.