"Нечитаемый" код?

Question

[sddvxd]

Добрый день! Делаю задание в учебнике, сказано написать функцию для "переворачивания" строки. Вспомнил про удобный оператор for, вот так можно им злоупотреблять и насколько понятен этот код?

char* rev(const char* str){
    int length = 0;
    const char* pchar = str;
    for(; *pchar; ++length, ++pchar);
    char* new_char = new char[length + 1];
    for(char* pnew_char = new_char; *pnew_char++ = *--pchar;); //pchar == '\0', поэтому префиксный --
    return new_char;
}

int main(int argc, char *argv[])
{
    cout << rev("hello") << '\n';
}

Comments

[Roman]

Сравните и сами поймете

https://ru.cppreference.com/w/cpp/algorithm/reverse

template<class BidirIt>
void reverse(BidirIt first, BidirIt last)
{
    while ((first != last) && (first != --last)) {
        std::swap(*first++, *last);
    }
}

в boost он же используется https://www.boost.org/doc/libs/1_68_0/boost/range/...

...

то что у вас задумано

https://ru.cppreference.com/w/cpp/algorithm/revers...

template<class BidirIt, class OutputIt>
OutputIt reverse_copy(BidirIt first, BidirIt last, OutputIt d_first)
{
    while (first != last) {
        *(d_first++) = *(--last);
    }
    return d_first;
}

аналогично первому https://www.boost.org/doc/libs/1_68_0/boost/range/...

[Василий Мельников]

И кстати условие
*pnew_char++ = *--pchar
неверное. С чего вдруг впереди строки окажется 0?

[devalone]

Понять то можно, но это ужасный код, не стоит так писать

[sddvxd]

Василий Мельников, после 5 итераций, pchar указывает на нуль-терминатор

for(char* pnew_char = new_char; *pnew_char++ = *--pchar;); //работает
 for(char* pnew_char = new_char; *pnew_char++ = *pchar--;); //не работает, *pnew_char == '\0'

[Василий Мельников]

sddvxd, в области констант что-то случайно оказалось. если заменить константную строку на динамическую или лежащую в стеке всё может поменяться, если в программе добавятся какие-то константы то тоже может сломаться.
суть в том, что ноль лежит в конце строки а не в начале.

[Василий Мельников]

sddvxd, например код

char str[] = "hello";
	cout << rev(str) << '\n';

У меня дал вывод

olleh╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠
╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠
╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠~¤р

Answer 1

[Василий Мельников]

Ну начнем с того, что строку можно перевернуть не выделяя дополнительной памяти. Во вторых возвращать из функции указатель на выделенный блок памяти без лишней необходимости не стоит. Иначе как минимум нужно будет сделать функцию освобождения этого блока.
Что касается циклов. Делать так можно, если никого в команде это не напрягает, но на мой взгляд не нужно. Просто современные компиляторы отлично оптимизируют код, и практической необходимости в подобных упражнениях нет, а ошибиться гораздо легче. Не зря в новые стандарты добавлены циклы по диапазонам и прочие плюшки.

Comments

[sddvxd]

почему не нужно выделять память? я же передаю строку, не объект. После завершения функции константа же должна удалиться

[Василий Мельников]

Я не говорю не нужно. Можно, обычно как раз в этом и заключаются подобные обучающие задачи

Answer 2

[Saboteur]

код плохо читаемый, потому что названия функций и переменных у вас плохо говорят для чего они используются.
А фор - ну технически можно и так.

Answer 3

[jcmvbkbc]

for(char* pnew_char = new_char; *pnew_char++ = *--pchar;);

Этот цикл работает неправильно, потому что выход из него происходит по воле случая, в зависимости от мусора, оказавшегося перед строкой, на которую указывает pchar.

for(; *pchar; ++length, ++pchar);

Делать так (писать цикл с пустым телом и точкой с запятой на той же строке) не стоит, потому что легко не заметить эту точку с запятой или её отсутствие.

for(char* pnew_char = new_char; *pnew_char++ = *--pchar;);

Так делать (писать тело цикла в его поле для условия выхода, равно как и писать его в последнем поле) не стоит, потому что всему своё место, а ошибку в этом цикле ты уже допустил.

Comments

[sddvxd]

не подскажете, где в последнем ошибка?

[sddvxd]

Этот цикл работает неправильно, потому что выход из него происходит по воле случая, в зависимости от мусора, оказавшегося перед строкой, на которую указывает pchar.

перед pchar не может быть мусора! Он указывает на нуль-терминатор переворачиваемой строки, а значит перед ним строка

[jcmvbkbc]

Он указывает на нуль-терминатор переворачиваемой строки, а значит перед ним строка

sddvxd, а в конце цикла, после того как обработан первый элемент переворачиваемой строки -- перед ним что?

[sddvxd]

jcmvbkbc, так pchar на начало цикла настроен на 6 символ (нулевой):

До цикла: *pchar == '\0'
Начало цикла:
*--pchar == 'o' == *pnew_char++ == первый элемент
*--pchar == 'l'
*--pchar == 'l'
*--pchar == 'e'
*--pchar == 'h'

[sddvxd]

jcmvbkbc, кажется, я понял о чем вы. исправил на такое:

char* rev(const char* str){
    int length = 0;
    const char* pointer_string = str;
    while(*pointer_string){
        length++;
        if(*(pointer_string+1)) pointer_string++;
        else break;
    }
    char* new_char = new char[length + 1];
    for(char* pnew_char = new_char; *pnew_char++ = *pointer_string--;);
    return new_char;
}

int main(int argc, char *argv[])
{
    cout << rev("hello") << '\n';
}

[sddvxd]

jcmvbkbc, теперь уж точно понял...

char* rev(const char* str){
    int length = 0;
    const char* pointer_string = str;
    while(*pointer_string){
        length++;
        if(*(pointer_string+1)) pointer_string++;
        else break;
    }
    char* new_char = new char[length + 1];
    char* pnew_char = new_char;
    for(int i = 0; i < length; ++i)
        *pnew_char++ = *pointer_string--;
    return new_char;
}

[jcmvbkbc]

while(*pointer_string){
        length++;
        if(*(pointer_string+1)) pointer_string++;
        else break;
    }

sddvxd, это можно заменить на куда более понятное и быстрое:

length = strlen(pointer_string);
pointer_string += length - 1;

Кроме того, два условия выхода из цикла (одно внутри while, другое перед break) -- это слишком много.