Why can't I increment a variable like this?

Pray tell. What is wrong with the unary-not operator?

2 Likes

Hardly readable unless used with a separating space, but more importantly, it applies to the last expression of a chain of method calls:

//   /-----------------------------+
//  V                              |
if  !elems //                      |
        .iter() //                 |
        .filter(...) //            |
        .all(...)  // <------------+
{
    ...
}

which is a pity since whenever ::core::ops::Not is in scope (which it could always automatically be, if such trait was in the prelude), you can write a suffix .not() instead, for a more readable chain of method calls.

4 Likes

Why is "Not" not in the prelude?

2 Likes

I find ! to be more readable in certain cases, especially when looking at a small condition:

if !self.worked() { 
    println!("Oh no!");
}

But I also find both .not() and ! to be odd when looking at data flow through a large statement:

let foo = my_str
    .chars()
    .map(|c|
        my_list
            .iter()
            .map(|x| {...})
            .any(|(y, z)| {...})
            .not()
    )
    .collect::<Vec<usize>>();

But it also makes it more readable when looking at the end of an expression like @yandros's example, where the application is at the very end.

Another thing that I prefer ! for is factoring out a ! in an expression:

let x = ![1, 2, 3, 4, 5, 6, 7, 8, 9]
    .iter()
    .any(|x| x < 5);
// Turns into
let x = [...]
    .iter()
    .all(|x| x >= 5); //Negation inline here.

I find pros and cons to both forms.

3 Likes

This last transformation should turn any into all

6 Likes

Good catch.

I believe it can be useful shorthand to allow only prefix increment / decrement to quickly increment / decrement a variable. The restriction / difference to C/C++ would be to make the operator return (), so that it can only ever be used to apply a side effect to one variable, not to index something and then increment the index in one line like C++ allows:

// reads value at old index, but after this line index is incremented
some_array[index++];

Also no use of it in complex expressions:

// Is y 2x or 2x + 1? Was the expression evaluated rtl or ltr?
y = x + x++;

It would be useful for situations like incrementing the length of a collection internally:

impl<T> SomeCollection<T> {
    pub fn push(&mut self, value: T) {
        ...
        ++self.len;
    }
}
1 Like

I'm not personally convinced that using ++self.len is more readable than self.len += 1 but even if it is, is it really worth adding a new operator just for that?

2 Likes

Well, it is readable for me, but maybe because I know C++. I think a lot of people who wrote C-like languages will also find the ++self.len more intuitive. But it isn't a dealbreaker either way :wink:

As someone who did a lot of C/C++ for over a decade, I'd wonder why ++self.len was used rather than self.len++ in a context where we don't care about the value. The best argument I see for the increment operators is when you really do want to return (or assign) the previous value while changing a the value in the variable, but I've yet to find myself reaching for that particular functionality since I've been using rust.

4 Likes

My C++ experience says the opposite: preferring post-increment is premature pessimization. Post-increment is never faster than pre-increment, it's no simpler to type than pre-increment, and pervasive use makes it harder to notice the places it was used intentionally.

9. Don’t pessimize prematurely

Easy on yourself, easy on the code: All other things being equal, notably code complexity and readability, certain efficient design patterns and coding idioms should just flow naturally from your fingertips and are no harder to write than the pessimized alternatives. This is not premature optimization; it is avoiding gratuitous pessimization.

9. Don’t pessimize prematurely - C++ Coding Standards: 101 Rules, Guidelines, and Best Practices [Book]

("Using postfix ++ when the prefix version is just as good." is in fact one of the examples.)

3 Likes

I'd like to make the following observation:

The auto increment and like operators are often use in C like languages to form loops and iterate over things in interesting ways. Here is a classic example that finds the next permutation after a given permutation:

boolean nextPermutation(int[] array) {
    int i = array.length - 1;
    while (i > 0 && array[i - 1] >= array[i])
        i--;
    if (i <= 0)
        return false;
        int j = array.length - 1;
    while (array[j] <= array[i - 1])
        j--;
    int temp = array[i - 1];
    array[i - 1] = array[j];
    array[j] = temp;
        j = array.length - 1;
    while (i < j) {
        temp = array[i];
        array[i] = array[j];
        array[j] = temp;
        i++;
        j--;
    }
    return true;
}

Here we see four uses of auto inc/dec operators and a lot of fiddling with array indexes.

Now, using the higer level features available in a language like Rust removes the need for any auto inc/dec and reduces the fiddling with array indexes a lot. Here is my attempt at the same problem in Rust:

fn nextPermutation <T: PartialOrd + Copy> (permutation: &Vec<T>) -> Option<Vec<T>> {
    if permutation.len() >= 1 {
        for i in (0..permutation.len() - 1).rev() {
            if  permutation[i] < permutation[i + 1] {
                for j in (0..permutation.len()).rev() {
                    if permutation[j] > permutation[i] {
                        let mut result: Vec<T> = permutation.clone();
                        result[i] = permutation[j];
                        result[j] = permutation[i];
                        result[(i + 1)..].reverse();
                        return Some(result)
                    }
                }
            }
        }
        return None;
    }
}

Notice how this removes a lot of manual array index bookkeeping and hence I think is a lot less prone to error.

I conclude that auto inc/dec is not much use in Rust, that not having it reduces programmer error and I would vote against trying to include it.

How this style compares with the typical C style performance wise I have yet to determine...I'll leave that as an exercise for the reader.

Java code courtesy of the wonderful Project Nayuki : https://www.nayuki.io/

2 Likes

Here is the more idiomatic version of next_permutation

fn next_permutation<T: PartialOrd + Copy>(permutation: &[T]) -> Option<Vec<T>> {
    use std::convert::TryFrom;
    
    for (i, i_val) in permutation.windows(2).enumerate().rev() {
        let [i_val, i_next] = <[T; 2]>::try_from(i_val).expect("Should Work");

        if i_val >= i_next {
            continue;
        }

        for (j, &j_val) in permutation.iter().enumerate().rev() {
            if j_val > i_val {
                let mut result = permutation.to_vec();

                result.swap(i, j);
                result[(i + 1)..].reverse();

                return Some(result);
            }
        }
    }

    None
}

And if you want it as a single iterator you have this monstrosity

fn next_permutation<T: PartialOrd + Copy>(permutation: &[T]) -> Option<Vec<T>> {
    use std::convert::TryFrom;
    // These renamings are for clarity
    use std::result::Result::{Err as Found, Ok as Continue};

    permutation
        .windows(2)
        .enumerate()
        .rev()
        .map(|(i, i_val)| (i, <[T; 2]>::try_from(i_val).expect("Should Work")))
        .filter(|(_, [i_val, i_next])| i_val < i_next)
        .try_fold((), |(), (i, [i_val, _])| {
            permutation
                .iter()
                .enumerate()
                .rev()
                .try_fold((), |(), (j, &j_val)| {
                    if j_val > i_val {
                        let mut result = permutation.to_vec();

                        result.swap(i, j);
                        result[(i + 1)..].reverse();

                        Found(result) // Err == Found means early return from try_fold
                    } else {
                        Continue(())
                    }
                })
        })
        .err() // Err == Found
}
4 Likes

KrishnaSannasi

If that is idiomatic Rust and the expected way to do things then I'm out of here right now!

Given that the algorithm to do this is:

  1. Find the largest index k such that a[k] < a[k + 1]. If no such index exists, the permutation is the last permutation.
  2. Find the largest index l greater than k such that a[k] < a[l].
  3. Swap the value of a[k] with that of a[l].
  4. Reverse the sequence from a[k + 1] up to and including the final element a[n].

I want code that sort of, somehow, looks like that and preferably understandable by someone not steeped in all the details of the programming language.

To that end I think I liked my first draft better, it has steps 1) and 2) spelled out as one for...{...} following another. Clearly demarking the steps and commented as such.

All that "?", "windows", "<[T; 2]>::try_from", "expect" stuff is superfluous to the algorithm an impenetrable by a casual reader.

I already thought my attempt at making this generic was turning into "line noise".

As for the "monster" version, one might as well write it in Forth or BF :slight_smile:

I really do think there is an abstraction to far...

1 Like

Ok, in that case here is a one-step towards idiomatic, I'll explain why I made each change as well

fn next_permutation<T: PartialOrd + Clone> (permutation: &[T]) -> Option<Vec<T>> {
    for i in (0..permutation.len() - 1).rev() {
        let next = &permutation[i + 1];
        let current = &permutation[i];
        
        if current >= next {
            continue
        }
        
        for (j, val) in permutation.iter().enumerate().rev() {
            if val > current {
                let mut result = permutation.to_vec();
                
                result.swap(i, j);
                result[(i + 1)..].reverse();
                
                return Some(result)
            }
        }
    }
    
    None
}
  1. name varaibles to make things more clear and to de-duplicate computation
  2. Change input to be &[T] to allow more stuff to use this function, like arrays
  3. remove redundent if permutation.len() >= 1, the range iterator handles that automatically
  4. use continue to reduce rightward drift, and make the code easier to read
  5. permutation.iter().enumerate() is very common, so let's use a common idiom to tell our intent
  6. use swap instead of doing it manually
    • This has the added benefit of now making T: Clone instead of T: Copy
  7. None instead of return None
5 Likes

Thanks for the examples and explanation.

No doubt I will find myself writing more rusty Rust as I get familiar more familiar with it all. Soon forgetting any ideas about legibility for the uninitiated.

Sadly neither that or the first (non-monster) version give the correct result. There is an off by one error such that an input of [1, 4, 3, 2] results in none rather than [2, 1, 3, 4]

My code does the same as yours,

So any correctness issues are not due to my transformations.


now fixed in previous posts

1 Like

Oops!

You see, I already had the bug and fixed it. There is a "(i > 0) &" in my original code above which should not be there!

Sorry.

I fixed it on the play pen.

ok, then changing the range to be (0..permutation-lens-1) should fix the issue, edited my previous posts to fix this issue

Yes that is better.

It now fails with an 'attempt to subtract with overflow' when given an empty vector.

Gotta love all the checking Rust does !

1 Like