Смотри! Дым! Кто у нас виноват?


Недавно я нашел способ генерировать конкретные вину за SmokeDetector. Это было круто, поэтому я решил сделать программу на C++ для этого. У меня тоже есть на Гитхабе. Эта программа называется валить бота.

Винить бота не нужны никакие флаги для компиляции, так что вы можете использовать make blame чтобы скомпилировать его.

Как это работает:

Виноват бот берет входной ряда. Этот вход имеет свой id чата. Это количество берется из вашего чата ссылке. Мой будет такой: https://chat.stackexchange.com/users/301688/grant-garrisonи мой номер будет 301688. Затем, число преобразуется к основанию 7. Затем оператор switch выводит символы. (Примечание: символы должны заключаться в двойные кавычки.)

#include <iostream>

int main(){
    int digit;
    std::string stringDigit, answer, instring;
    std::cin>>instring;
    int in = std::stoi(instring,nullptr);
    while (in != 0){
        digit = in % 7;
        if (digit < 10){
            stringDigit = '0' + digit;
        }else{
            stringDigit = digit - 10 + 'A';
        }
            answer = stringDigit + answer;
            in /= 7;
    }
    std::cout<<"!!/blame\u180E ";
    int i = 0;
    while(answer.length()>=i){
        switch (answer[i]-'0'){
            case 0: std::cout<<"\u180E";
                break;
            case 1: std::cout<<"\u200B";
                break;
            case 2: std::cout<<"\u200C";
                break;
            case 3: std::cout<<"\u200D";
                break;
            case 4: std::cout<<"\u2060";
                break;
            case 5: std::cout<<"\u2063";
                break;
            case 6: std::cout<<"\uFEFF";
                break;
        }
        i++;
    }
    std::cout<<"a\n";
}

Что я ищу:

Я ищу эффективности и улучшения синтаксиса. Я знаю, что программа уже быстро, но каких-либо дополнительных вычислений вырезать бонус. Я хочу оставить { и } на if заявления, хотя, как я думаю, что это улучшает читаемость для меня. Я также не хочу использовать любые библиотеки.

Что я думаю, что может быть улучшено:

Я думаю, что переключаться из std::string и int является неэффективной, и я уверен, что есть лучший способ это не нужно. Я тоже думаю, что алгоритм преобразования базы есть лучшая замена.



1098
8
задан 7 апреля 2018 в 03:04 Источник Поделиться
Комментарии
4 ответа

Расстояние

Использование (по горизонтали) пространство не последовательны, у вас есть пространство
после if и switch, но не после else и while.
Я бы вообще больше использовать пробелы для улучшения читаемости, в особенности после того, как ключевые слова, вокруг операторов и т. д. Например

while (answer.length() >= i) {

Область

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

Проверка ввода

std::stoi(instring, nullptr) молча игнорирует ведущие пробелы
и трейлинг номера-цифры, которые могут или не могут быть нужны.
В pos параметр может использоваться, чтобы проверить, если вся строка
была успешно преобразована.

Также std::stoi(instring, nullptr) бросает std::invalid_argument
исключение, если входная строка не содержит цифр действует на всех,
вы, возможно, захотите, чтобы поймать это.

Слишком далеко переборе

В

int i = 0;
while(answer.length()>=i){
switch (answer[i]-'0'){
// ...
}
i++;
}

последней итерации обращается answer[answer.length()], который
нулевой символ. Даже если это причиняет никакого вреда (answer[i]-'0'
тогда не совпадает ни с одним из случаев) это, вероятно, непредвиденным, и вы будете
хочу answer.length() > i вместо. (См. ниже для дальнейших улучшений
из этой части.)

Упрощений

Остаток по модулю 7 не может быть >= 10, так что цикл while
упрощается

while (in != 0) {
int digit = in % 7;
std::string stringDigit;
stringDigit = '0' + digit;
answer = stringDigit + answer;
in /= 7;
}

что в дальнейшем может быть упрощен до

while (in != 0) {
char digit = '0' + in % 7;
answer.insert(0, 1, digit);
in /= 7;
}

делая stringDigit переменная устарели, и мутирует answer
вместо конкатенации строк.

Перебора всех допустимых строковых индексов лучше сделать с помощью цикла for:

for (int i = 0; i < answer.length(); ++i) {
// ... use `answer[i]` ...
}

Но на самом деле вы не нуждаетесь в индекс, только символы, и
перебора всех символов в строке можно сделать более просто
с набором для петля (добавлен в C++ 11):

for (char& c : answer)
// ...
}

Оператор switch может быть заменен подстановки массива:

std::string blameStrings[] = {
"\u180E", "\u200B", "\u200C", "\u200D", "\u2060", "\u2063", "\uFEFF"
};
for (char& c : answer) {
std::cout << blameStrings[c - '0'];
}

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

Избавиться от промежуточного хранилища строк

Ваш код вычисляет answer в качестве базового-7 представление
номер входа, а затем перебирает символы в этой строке.

Преобразование из цифры в символы в строку и обратно
цифры можно избежать путем хранения целых чисел в вектор
вместо. И вместо добавления дополнительных цифр, мы можем добавить
их (чтобы избежать что должен памяти быть несколько раз переехал), а затем перебирать
за результат в обратном порядке:

#include <vector>

// ...

int in = std::stoi(instring, nullptr);
std::vector<int> digits;
while (in != 0) {
digits.push_back(in % 7);
in /= 7;
}

std::cout<<"!!/blame\u180E ";
for (auto it = digits.rbegin(); it != digits.rend(); ++it) {
std::cout << blameStrings[*it];

}
std::cout<<"a\n";

11
ответ дан 7 апреля 2018 в 08:04 Источник Поделиться

    int digit;
std::string stringDigit, answer, instring;
std::cin>>instring;
int in = std::stoi(instring,nullptr);

Объявлять переменные, как вы нуждаетесь в них.

Каждый файл должен быть самодостаточным. Убедитесь, что вы #include заголовки для компонентов, которые вы используете. Не полагайтесь на неявное включение.

Проверьте, что std::cin успешно прочитать значение из потока.

А не прыгать через обруч std::stoi преобразования, просто прочитать значение как целое число.

#include <iostream>
#include <string>

int main() {
int in;
if (!(std::cin >> in)) {
std::cerr << "Invalid user-id";
return EXIT_FAILURE;
}


    while (in != 0){
digit = in % 7;
if (digit < 10){
stringDigit = '0' + digit;
}else{
stringDigit = digit - 10 + 'A';
}
answer = stringDigit + answer;
in /= 7;
}

Используйте символические константы вместо магических констант ('0', 'A', 7).

Если вы принимаете модуль 7, digit никогда не будет больше 7. В else филиал никогда не принимается.


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

Вы найдете остатки, отформатируйте их в ASCII, восстановить их обратно в остатки (индексы), затем отформатировать их в свой собственный формат. Просто сохраните промежуточных остатков и карту к своим ценностям.

#include <algorithm>
#include <array>
#include <iostream>
#include <iterator>
#include <vector>

int read_int(std::istream& in) {
// ...
}

std::vector<int> itoa_unformatted(int number, int base) {
// ...
}

int main() {
constexpr auto mappings = std::make_array("\u180E", "\u200B", "\u200C", "\u200D",
"\u2060", "\u2063", "\uFEFF");

auto user_id = read_int(std::cin);
auto digits = itoa_unformatted(user_id, mappings.size());
transform(digits, std::make_ostream_joiner(std::cout, ""),
[&mappings](auto index) { return mappings[index]; });
std::cout << "a\n";
}

Примечание - пример кода для экспозиции только. std::make_array и std::make_ostream_joiner являются частью языка C++20 (библиотека основы П2). transform это всего лишь помощник для std::transform над контейнером. Если вы хотите играть с этими функциями теперь при помощи GCC/Clang в, см. mnmlstc ядра.

Если ваша база не изменится, вам не нужно обобщенного решения преобразования базы тогда. Счисления могут быть специализированными для вашего конкретного случая использования, от размера соответствующего буфера, чтобы сила сокращения дорогостоящих операциях (например, отдел/модуль). Всегда измерения перед оптимизацией.

5
ответ дан 7 апреля 2018 в 09:04 Источник Поделиться

Я отправляю это, чтобы дать полный круг резюме улучшение, плюс я нашел несколько улучшений самого себя, но помните, они в основном не мои усовершенствования. Голосовать за другие ответы! Этот код был сокращен до почти 1/3 своего размера!

Окончательный код:

#include <iostream>
#include <string>

int main(){
int in;
std::string answer;
std::cin>>in;
std::string blameStrings[] = {
"\u180E", "\u200B", "\u200C", "\u200D", "\u2060", "\u2063", "\uFEFF"
};
while (in != 0) {
answer.insert(0,blameStrings[(in % 7)]);
in /= 7;
}
std::cout<<"!!/blame\u180E "<<answer<<"a\n";
}

Построчно:

Я только собрался объяснить код, который я изменил. Вы можете посетить (и upvote) другие ответы для их объяснения.

#include <iostream>
#include <string>

Дополнительные #include было предложено Snowhawk.

int main(){
int in;
std::string answer;
std::cin>>in;

Удаление дополнительных переменных был предложен как Snowhawk и Мартин Р. снятие stoi было предложено Snowhawk. Я также лично сняты несколько дополнительных переменных.

    std::string blameStrings[] = {
"\u180E", "\u200B", "\u200C", "\u200D", "\u2060", "\u2063", "\uFEFF"
};

Массив был предложен Р. Мартин

    while (in != 0) {
answer.insert(0,blameStrings[(in % 7)]);
in /= 7;
}

Эта часть кодекса была выдвинута Р. Мартина, однако, я увидел, что есть цикл перебора, что первый цикл был создан, так что я пропила второй цикл.

    std::cout<<"!!/blame\u180E "<<answer<<"a\n";
}

Это просто выводит все в один простой вызов std::cout.

Помните, что большинство это не моя улучшений, поэтому убедитесь, что Вы проголосовать за ответы, что улучшения появились.

3
ответ дан 7 апреля 2018 в 08:04 Источник Поделиться

Отсутствует заголовок

Мы должны включить <string> чтобы получить декларации для std::string и std::stoi.

Компилируется с предупреждениями включен

Вы говорите, что не нужно каких-либо флагов компиляции. Несмотря на то, что это правда (при условии по умолчанию компилятор для C++11 или более поздней версии), лучше включить некоторые предупреждающие флаги. Что помогает выявить дефекты, такие как этот:

g++ -std=c++17 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Weffc++ 191455.cpp -o 191455
orig.cpp: In function ‘int main()’:
orig.cpp:20:26: warning: comparison of integer expressions of different signedness: ‘std::__cxx11::basic_string<char>::size_type’ {aka ‘long unsigned int’} and ‘int’ [-Wsign-compare]
while(answer.length()>=i){
~~~~~~~~~~~~~~~^~~

Хотя несоответствие не вызовет ошибку, она может быть легко исправлена путем простого изменения i в беззнаковый тип. Тогда будущие читатели могут быть более уверены в своем коде, если ты не оставишь ловушек, таких как это.

Просто читать из входного

Там мало что можно получить, прочитав в строку, а затем вызвать std::stoi() без проверки конечного указателя. Просто поток прямо на целое число.

Не писать свой собственный код, чтобы преобразовать в восьмеричное

Вот почему мы имеем std::oct поток манипулятора.

Не добавить '0' чтобы включить цифры на цифры, а затем вычесть его снова

Либо изменить case для переключения на цифры (например, case '0':и т. д.) Или работать напрямую с малыми целыми числами. Я предпочитаю последний, так как мы можем использовать простой массив, чтобы посмотреть выходные символы.

Использовать for чтобы показать перебором элементов

Рассмотрим этот цикл:

int i = 0;
while(answer.length()>=i){
//...
i++;
}

Это более идиоматически записать в виде:

for (int i = 0;  i <= answer.length();  ++i)

Офф-по одной ошибке становится более очевидным. Лучше избегать этого перебором символов:

for (auto c: answer)

Не использовать узкие потоки для широких символов

Если мы печатаем широкий-символьных строк, мы должны писать в std::wcout вместо того чтобы std::cout.

Добавить некоторые модульные тесты

Достаточно сказал.


Замена кода

Без уточнения, я не могу проверить, является ли вывод правильный, но это производит тот же самый вывод, а исходный код для данного теста (301688), как показано с помощью встроенного самотестирования.

#include <string>
#include <vector>

std::wstring encode(unsigned long userid)
{
static wchar_t const code[7] = {
L'\u180E', // 0 => MONGOLIAN VOWEL SEPARATOR
L'\u200B', // 1 => ZERO WIDTH SPACE
L'\u200C', // 2 => ZERO WIDTH NON-JOINER
L'\u200D', // 3 => ZERO WIDTH JOINER
L'\u2060', // 4 => WORD JOINER
L'\u2063', // 5 => INVISIBLE SEPARATOR
L'\uFEFF', // 6 => ZERO WIDTH NO-BREAK SPACE
};

std::vector<wchar_t> octalid;
for (auto i = userid; i; i/=7) {
octalid.push_back(code[i % 7]);
}

return L"!!/blame\u180E "
+ std::wstring(octalid.rbegin(), octalid.rend())
+ L"a\n";
}

#include <iostream>
#include <locale>

int main(int argc, char **argv)
{
if (argc == 1) {
// no arguments - run self tests
return
+ (encode(0) != L"!!/blame\u180E a\n")
+ (encode(1) != L"!!/blame\u180E \u200Ba\n")
+ (encode(6) != L"!!/blame\u180E \uFEFFa\n")
+ (encode(7) != L"!!/blame\u180E \u200B\u180Ea\n")
+ (encode(8) != L"!!/blame\u180E \u200B\u200Ba\n")
+ (encode(301688) != L"!!/blame\u180E \u200C\u200D\uFEFF\u2060\u200D\uFEFF\u200Ca\n");
}

std::locale::global(std::locale{""});

for (int i = 1; i < argc; ++i) {
try {
std::size_t pos;
unsigned long userid = std::stoul(argv[i], &pos);
if (argv[i][pos])
throw std::invalid_argument("not a number");
std::wcout << encode(userid);
} catch (std::exception& e) {
std::cerr << "Invalid argument: " << argv[i]
<< ": " << e.what() << std::endl;
}
}
}

2
ответ дан 9 апреля 2018 в 10:04 Источник Поделиться