Rust implementation of the jarvis march algorithm

[jdonszelmann]

No description provided.

[berquist]

Thanks for the contribution! Just some minor things.

[berquist]

Nice idea to use an option instead of returning an empty vec, which means something other than "couldn't make the hull". Do you need to take ownership of gift though?

[jdonszelmann]

Not necessarily, good point and will be changed :)

[berquist]

This is an obvious comment that doesn't add much.

[jdonszelmann]

Sure, I'll remove it

[jdonszelmann]

@berquist I saw that in another implementation of Jarvis (I believe maybe Julia?) someone wanted this function to return an integer because the text says so. Should I do this in rust too?

[jdonszelmann]

I remember, it was in the Graham scan in rust (#479 )

[jdonszelmann]

Or maybe an enum?

[berquist]

For Jarvis March, these implementations have a function that returns a float:

• C
• Haskell
• Java

and these implementations have a function that returns a boolean:

• C#
• C++ (inline lambda)
• Common Lisp
• Go
• JavaScript
• Python
• Vlang

and these implementations do something else:

• Julia

As for which should be used, being consistent is best. I made that comment about returning a number in Graham Scan because of what the text describes, but here there is no such description in the text, despite this part of the algorithm being the same.

Regarding the enum, what advantage would that bring? This is really about getting an ordering between points, which you represent either as

1. < 0, == 0, or > 0 for two points, or
2. true/false for some condition, say p1 <= p2.

My response is that either a bool or a float is fine, and if we decide to standardize later on one or the other, it's an easy change.

[jdonszelmann]

Regarding floats, some algorithms (including this one, but also c# and V) can't even deal with floating point input coordinates. Any thoughts about that?

[jdonszelmann]

And for the enum, in rust it's not uncommon to use an Ordering as a return type here (https://doc.rust-lang.org/stable/std/cmp/enum.Ordering.html)

[berquist]

Ideally the allowed input is any number type (maybe not complex), but if it's restricted to just ints or floats, it's fine as long as the code is correct. If you want to make Point generic over Signed + Float, that's up to you.

If you can write a clean version that uses Ordering, that would be cool.

[jdonszelmann]

Ordering won't be clean. I'll just leave it at a boolean. I tried to convert the code to allow for floats, but this really messes the clarity up a lot because in rust, floats don't implement Ord and Eq

[jdonszelmann]

The part where I find the leftmost point would change from a simple min_by_key() to this:

    let leftmost_point = gift
        // Iterate over all points
        .iter()
        // Find the point with minimum x
        // Unfortunately, we can't use min_by_key here because floats in rust
        // do not implement Ord.
        .fold((f64::NAN, f64::NAN), |acc, val| if acc.0.partial_cmp(&val.0) == Some(Ordering::Less){
            acc
        } else {
            val
        })
        // If there are no points in the gift, there might
        // not be a minimum. Unwrap fails (panics) the program
        // if there wasn't a minimum, but we know there always
        // is because we checked the size of the gift.
        .unwrap()
        .clone();

[jdonszelmann]

🎉