Я пытался практиковать итераторы C++, определяя очень распространенный алгоритм сортировки слиянием, когда столкнулся с непоследовательным поведением программы. Мой вопрос заключается НЕ в том, как реализовать сортировку слиянием (которую я знаю и понимаю, есть много примеров ответов), а скорее в том, что при создании рекурсивных векторов с использованием итераторов я получаю противоречивые конечные результаты.
Я знаю, что могу решить проблему, скопировав значения в массивы L и R через цикл for, но я хотел бы понять, почему использование итераторов не работает. Это работает на CLION с C++ 14. Этот пост Лучший способ извлечь подвектор из вектора? не ответил на мой вопрос, так как я создаю вектор, аналогичный предписанным методам.
void merge2(vector<int> &arr, int l, int m, int r)
{
vector<int> L{arr.begin()+l, arr.begin()+m+1};
vector<int> R{arr.begin()+m+1,arr.begin()+r+1};
int i = 0, j = 0, k = l;
int n1 = m - l + 1;
int n2 = r - m;
while (i < (n1) && j < (n2)){
if (L[i]<=R[i]){ //R[i] is replaced by R[j]
arr[k] = L[i++];
}
else {
arr[k] = R[j++];
}
k++;
}
while (i < n1)
{
arr[k] = L[i];
i++;
k++;
}
while(j < n2){
arr[k] = R[j];
j++;
k++;
}
}
/* l is for left index and r is right index of the
sub-array of arr to be sorted */
void merge2Sort(vector<int> &arr, int l, int r)
{
if (l < r)
{
// Same as (l+r)/2, but avoids overflow for
// large l and h
// int m = (l+r-l)/2;
// int m = l+(r-l)/2;
int m = (l+r)/2;
// Sort first and second halves
merge2Sort(arr, l, m);
merge2Sort(arr, m+1, r);
merge2(arr, l, m, r);
}
}
int main(int argc, char **argv){
vector<int> arr = {12, 11, 13, 5, 6, 7};
merge2Sort(arr, 0, arr.size()-1);
for(auto i = arr.begin(); i != arr.end(); i++){
cout << *i << " ";
}
return 0;
}
Иногда я получаю правильный ответ 5 6 7 11 12 13, а иногда неправильный ответ 5 6 7 11 13 12. Ответ не должен меняться в зависимости от попытки.
Правильный ответ, чтобы отразить ответ и комментарии. Ответ исправляет ошибку индексации и полагается на итераторы. Также из комментариев следует отметить, что векторы при инициализации из итераторов должны использовать (), а не {}.
template <class It>
void merge(It left, It middle, It right)
{
// Beeing generic, you need to retrieve the type.
using value_type = typename std::iterator_traits<It>::value_type;
// You can copy only the first half
std::vector<value_type> left_side_copy(left, middle);
It L = left_side_copy.begin();
It R = middle;
while (L != left_side_copy.end() && R != right)
{
if ( *L <= *R )
{
*left = *L;
++L;
}
else {
*left = *R;
++R;
}
++left;
}
// Copy only the leftovers, if there are any
std::copy(L, left_side_copy.end(), left);
}
template <class It>
void merge_sort(It left, It right)
{
if (auto dist = std::distance(left, right); dist > 1)
{
It middle = left + dist / 2;
merge_sort(left, middle);
merge_sort(middle, right);
merge(left, middle, right);
}
}
int main(void)
{
std::vector<int> arr {
5, 12, 11, 13, 5, 4, 7, 13, 6
};
merge_sort(arr.begin(), arr.end());
for(auto const &i : arr) {
std::cout << ' ' << i;
}
std::cout << '\n';
}
vector<int> L{arr.begin()+l, arr.begin()+m+1};
должно быть vector<int> L(arr.begin()+l, arr.begin()+m+1);
. Фигурные скобки означают предпочтение выбора конструктора на основе содержимого списка, являющегося содержимым вектора (а не пары итераторов). Если ничего не может быть найдено первым способом, он возвращается к рассмотрению пары итераторов, но вы всегда рискуете случайно вызвать первый способ.
Ошибку должно быть очень легко найти, если вы поместите некоторую функцию дампа для подмассива при каждом вызове сортировки слиянием.
Спасибо, М.М., я изменил конструкторы, чтобы использовать предложенный метод () вместо {} для инициализации вектора.
Спасибо, @Ayxan Как вы определили, что исходная неисправленная программа при отладке выдавала за пределы допустимого диапазона? Я использую CLION, и предупреждения не было. Я бы исключил индекс выхода за пределы, чтобы выдать ошибку.
@ user4242176 VisualStudio выдает ошибку вне допустимого диапазона для operator[]
в режиме отладки
Хорошо, похоже, это функция VisualStudio. Обычно мне приходится использовать .at(), чтобы увидеть ошибку индекса за пределами границ в CLION. Интересно.
@ user4242176 Почему вы отредактировали вопрос, чтобы отразить ответ? Это делает ответ совершенно излишним для того, кто увидит его позже.
@Ayxan, я пробовал, но не похоже, или, по крайней мере, я не знаю. Например, vector<int> arr = {12,11,13,5,6,7} ; int i2 = arr[99];
выдает значение i2, равное 0. В C доступ к элементу массива за пределами границ затухает до случайного значения из памяти, и я ожидаю такого поведения, но не от C++. CLION компилирует, запускает и выводит значение 0 для i2.
Кстати, если вы практикуете итераторы, вам следует использовать итераторы использовать;).
@Bob__ хорошо, и спасибо, я обновил ответ, чтобы отразить ссылку. Это очень идиоматический итератор С++, я просто предполагаю, что вы часто используете С++. ;)
Вы используете индекс i
вместо j
в векторе R
. Замените R[i]
на R[j]
см. ниже
void merge2(vector<int> &arr, int l, int m, int r)
{
//..
while (i < (n1) && j < (n2)){
if (L[i]<=R[j]){ //R[i] is replaced by R[j]
arr[k] = L[i++];
}
else {
arr[k] = R[j++];
}
k++;
}
//...
}
Спасибо, хотя это и исправляет мою пропущенную ошибку в коде, непоследовательное поведение остается. Так что я не думаю, что это полностью решает вопрос.
@ user4242176 ты уверен? Я получаю правильный ответ каждый раз. изменить индекс и пересобрать программу
Строка int m = l+(r-l)/2;
неверна. Вы, наверное, имели в виду int m = (l+r-l)/2;
, хотя я думаю, что лучше оставить int m = (l+r)/2;
.
Спасибо, я не верю, что это решенная проблема. Попытка предложенного изменения приводит к ошибке с кодом 11.
Это дает ошибку вне диапазона при отладке, что, вероятно, приводит к тому, что UB находится в режиме выпуска.