Что нужно исправить в этом коде, чтобы из глаз не шла кровь?

Question

[Conan_Doyle]

Решил составить программу, которая работает по следующему алгоритму:

Некоторый алгоритм из одной цепочки символов получает новую цепочку следующим образом. Сначала вычисляется длина исходной цепочки символов; если она чётна, то в середину цепочки символов добавляется символ А, а если нечётна, то в начало цепочки добавляется символ Б. В полученной цепочке символов каждая буква заменяется буквой, следующей за ней в русском алфавите (А — на Б, Б — на В и т. д., а Я — на А). Получившаяся таким образом цепочка является результатом работы алгоритма.

Например, если исходной была цепочка ВРМ, то результатом работы алгоритма будет цепочка ВГСН, а если исходной была цепочка ПД, то результатом работы алгоритма будет цепочка РБЕ.

Дана цепочка символов ТОР. Какая цепочка символов получится, если к данной цепочке применить описанный алгоритм дважды (т. е. применить алгоритм к данной цепочке, а затем к результату вновь применить алгоритм)? Русский алфавит: АБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯ.

(взято из ОГЭ по информатике)

Что тут можно улучшить? За что стоит отобрать клавиатуру?

#include<iostream>
using namespace std;

int main(){
    //исходная строка и новая
    string str = "TOP", newStr = "";

    if(str.length()%2) //если нечётная, то в начало добавляем В
        newStr = "B"+str;
    else{ //иначе в середину А
        for(int i = 0; i < str.length()/2; i++)
             newStr+=str[i];

        newStr+="A";

        for(int i = str.length()/2; i < str.length(); i++)
            newStr+=str[i];
    }

    //заменяем буквы
    for(int i = 0; i < newStr.length(); i++){
        newStr[i] = newStr[i]+1;
    }
    //output: CUPQ
    cout << newStr << endl;

    return 0;
}

(русские буквы заменил на английские)

Comments

[GavriKos]

Я бы за фигурные скобки руки вырвал. Где то есть, где то нету, и везде на той же строке а-ля JS.
Но это субъективно, и не имеет отношения к решению алгоритма.

[Antony]

GavriKos, Все же, чтобы из глаз не шла кровь (а автор именно так сформулировал) нужно скобки ставить как в C++, а не как в Java

[Евгений Шатунов]

Antony, а объясни пожалуйста, как в C++ нужно ставить фигурные скобки?

[Antony]

Евгений Шатунов,

int main()
{
    for(;;)
    {
        if(true)
        {
        }
    }
}

[Antony]

Еще замечу автором что писать вот так:

if(str.length()%2) //если нечётная, то в начало добавляем В
        newStr = "B"+str;

ИМХО не самая хорошая идея - если вам придет в голову завтра вставить туда еще что-то есть риск, что вы забудете взять этот блок кода в фигурные скобки. Так что лучше сразу их писать. Это исключительно мое мнение. лично видел когда хорошие спецы с такого кода огорчались, ровно как и наоборот видел как хорошие спецы писали вот так.

Answer 1

[terrier]

Давайте подумаем вместе:
- Разумно ли добавлять к строке по одному символу с точки зрения производительности?
- Можем ли мы решить задачу за один проход по итоговому массиву, а не за два в худшем случае как у вас сейчас?
- Знаем ли мы заранее длину итоговой строки?
- Можем ли мы в C++ создать строку заранее заданной длины?
- Есть ли в современном C++ способы пройтись по строке и трансформировать ее не используя явно дополнительные сущности типа индексов?
- Можем ли мы сделать так, чтобы программа работала для динамически задаваемых входных данных, а не для вкомпиленных?
Кроме того - у вас серьезная ошибка в этой строчке:
newStr[i] = newStr[i]+1;
перечитайте условие.

Answer 2

[jcmvbkbc]

Что тут можно улучшить?

std::string::insert вместо побуквенного перекладывания.
newStr[i]+1 не работает для 'Я'->'А' и вообще сильно от кодировки зависит.

Answer 3

[Roman]

улучшай(те)

#include <iostream>
#include <string>
#include <algorithm>
#include <windows.h>

void prepare(std::string& value, char a, char b)
{
  size_t sz = value.size();
  if(sz % 2)
    value.insert(0, 1, b);
  else
    value.insert(sz / 2, 1, a);
}

auto shift = [](const char c, const char a)
{
  return (c != 'Ё') 
         ? (c != 'Е') 
           ? a + ((c - a + 1) % 32)
             : 'Ё' 
               : 'Ж';
};


std::string& process(std::string& s, const char a, const char b, int repeat)
{
  if(!repeat) return s;
  prepare(s, a, b);
  std::transform(s.begin(), s.end(), s.begin(),
    [&](char c){return shift(c, a);
  });
  return process(s, a, b, repeat - 1);
};

int main()
{
  SetConsoleOutputCP(1251);
  std::string one("ВРМ");
  std::string two("ТОР");
  std::cout << process(one, 'А', 'Б', 1) << "\n"  // ВГСН
            << process(two, 'А', 'Б', 2) << std::endl; // ГФБРТ
}

Comments

[jcmvbkbc]

Да тоже нифига подобного: многократное применение относится ко всему алгоритму, а не только к замене букв. И если прочитать задание внимательно, то можно увидеть в русском алфавите букву Ё, которая в 1251 совсем не после E.

[Roman]

jcmvbkbc, возможно автор сам справится, там делов на пять минут. Хотя с кодировкой накладка. Но это тоже поправимо.

return (c != 'Ё') ? a + ((c - a + 1) % 32) : 'Ж';

std::string& process(std::string& s, const char a, const char b, int repeat)
{
  if(!repeat) return s;
  prepare(s, a, b);
  std::transform(s.begin(), s.end(), s.begin(),
    [&](char c){return a + ((c - a + 1) % 32);
  });
  return process(s, a, b, repeat - 1);
};

[jcmvbkbc]

return (c != 'Ё') ? a + ((c - a + 1) % 32) : 'Ж';

И всё ещё нет, но теперь с привязкой к cp1251.

spoiler

Должно быть два специальных случая

[Roman]

jcmvbkbc, ??? Сколько подсказок осталось.

[jcmvbkbc]

char next_char(char c)
{
    const char *alpha = "АБВГДЕЁЖЗ...Я";
    const char *p = strchr(alpha, c);
    return p ? (p[1] ? p[1] : *alpha) : c;
}

[Roman]

jcmvbkbc, Ок) 'Е' Но с костылем то же нормально.