Как сделать код этого алгоритма ржавчины чище?

Я перевожу это следующая перестановка алгоритм в C ++:

template<class BidirIt>
bool next_permutation(BidirIt first, BidirIt last)
{
    if (first == last) return false;
    BidirIt i = last;
    if (first == --i) return false;
  
    while (true) {
        BidirIt i1, i2;
  
        i1 = i;
        if (*--i < *i1) {
            i2 = last;
            while (!(*i < *--i2))
                ;
            std::iter_swap(i, i2);
            std::reverse(i1, last);
            return true;
        }
        if (i == first) {
            std::reverse(first, last);
            return false;
        }
    }
}

В этот код Rust:

struct Solution;
impl Solution {
    pub fn next_permutation(nums: &mut Vec<i32>) -> bool{
        if nums.len() < 2 {
            return false;
        }
        let first = 0;
        let last = nums.len();
        let mut i = last - 1;
        loop {
            let i1 = i;
            i -= 1;

            if nums[i] < nums[i1] {
                let mut i2 = last;

                loop {
                    i2 -= 1;
                    if nums[i] < nums[i2] {
                        break;
                    }
                }
                let tmp = nums[i];
                nums[i] = nums[i2];
                nums[i2] = tmp;
                nums[i1..last].reverse();
                return true;
            }
            if i == first {
                nums[first..last].reverse();
                return false;
            }
        }
    }
}

Хотя это работает, я думаю, что это не очень красиво. Я новичок в ржавчине, любые улучшения приветствуются!

1 ответ
1

В произвольном порядке:

  • Создавайте тестовые примеры, чтобы быть уверенным при рефакторинге.

  • Не создавайте пустые структуры (Solution) просто чтобы дать им функции. next_permutation отлично подходит как бесплатная функция. если ты имеют чтобы создать плохой API, удовлетворяющий LeetCode или что-то еще, напишите хорошо Сначала API, а затем оберните его. Если требуемый API настолько плох, что вы не можете обернуть его хорошим API, это, вероятно, пустая трата времени.

  • Использовать rustfmt.

  • Принимать &mut [T] вместо &mut Vec<T> когда менять размер не нужно.

  • let first = 0;
    let last = nums.len();
    

    Не давайте ненужных символических имен вещам, которые не используются символически. 0 гораздо более очевидно индекс первого элемента в срезе, чем first. Кроме того, вам даже не нужно указывать границы среза при индексировании с помощью [..]; они подразумеваются. Так что они вам нужны даже меньше, чем вы, возможно, думали.

  • i, i1, i2 бессмысленны. Как насчет описательных имен?

  • Я предпочитаю ставить каждый loop в блоке с его переменными состояния, чтобы ограничить область изменчивости.

  • let i1 = i;
    

    Аналогично примечанию на first а также last выше, не создавайте переменные, которые можно тривиально вычислить из других переменных. Отношение между i а также i + 1 намного очевиднее, чем i а также i1.

  • if nums[i] < nums[i + 1] { ... }
    

    Почему это тело вложено во внешний цикл? Алгоритм довольно легко разбивается на четыре этапа:

    1. Получить индекс самой правой восходящей пары значений
    2. Получить индекс самого правого значения больше, чем первый элемент этой пары
    3. Поменяйте местами значения этих двух индексов
    4. Переверните срез, начиная сразу после первого индекса. У вас есть 2, 3 и 4, вложенные внутрь 1, что делает алгоритм более сложным, чем он есть на самом деле.
  • let tmp = nums[i];
    nums[i] = nums[i2];
    nums[i2] = tmp;
    

    Использовать nums.swap(i, i2) вместо.

Применяемый

pub fn next_permutation(nums: &mut [i32]) -> bool {
    if nums.len() < 2 {
        return false;
    }
    let last_ascending_pair = {
        let mut i = nums.len() - 1;
        loop {
            i -= 1;

            if nums[i] < nums[i + 1] {
                break i;
            }
            if i == 0 {
                nums.reverse();
                return false;
            }
        }
    };

    let last_greater_than_n = {
        let mut i = nums.len();
        loop {
            i -= 1;
            if nums[last_ascending_pair] < nums[i] {
                break i;
            }
        }
    };
    nums.swap(last_ascending_pair, last_greater_than_n);
    nums[last_ascending_pair + 1..].reverse();
    true
}

Вы можете подумать, что есть более «итеративный» способ написать это, и вполне может быть. Я попробовал, но я не люблю результат. Я думаю, выбери свой яд.

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *