Тема: Оцініть, будь ласка, код на С++. Дайте поради

Дебажив код близько 2 годин, а може і більше. Не певен і зараз, чи повністю вірно функціонує программа. Чи може це бути пов'язано з тим, як я написав код? Чи може є інші проблеми? Може погано розумію C++ або якісь конкретні теми, що не пов'язані з конкретною мовою програмування. Можете, будь ласка, дати поради, виходячи з того коду, що я написав.
Программа проста. Отримує на вхід певний вираз, що складається з послідовностей та операції множення(конкатенація). Отриманий вираз программа обчислює та виводить результат — послідовність. Пробіли ігнорує.
Посилання на гугл диск з файлами проєкту: https://drive.google.com/file/d/1PRk-uF … sp=sharing

2

Re: Оцініть, будь ласка, код на С++. Дайте поради

Будь ласка, користуйтеся GitHub або схожими вебсервісами для шерінгу вашого коду:

До того ж схоже, у вас не багато файлів (.hpp/.cpp), тому можете їх викласти тут

Прихований текст

просто сховавши під спойлер

EpsilonMordecai написав:

Дебажив код близько 2 годин, а може і більше. Не певен і зараз, чи повністю вірно функціонує программа. Чи може це бути пов'язано з тим, як я написав код? Чи може є інші проблеми? Може погано розумію C++ або якісь конкретні теми, що не пов'язані з конкретною мовою програмування. Можете, будь ласка, дати поради, виходячи з того коду, що я написав.
Программа проста. Отримує на вхід певний вираз, що складається з послідовностей та операції множення(конкатенація). Отриманий вираз программа обчислює та виводить результат — послідовність. Пробіли ігнорує.

Опишіть детальніше, що, на вашу думку, ваш код робить не так. Що саме ви вводите і, що отримуєте. А, що очікували отримати. Також опишіть, що ви вже пробували робити. Це допоможе нам, допомогти вам.

Подякували: lucas-kane, EpsilonMordecai2

3

Re: Оцініть, будь ласка, код на С++. Дайте поради

EpsilonMordecai написав:

Дебажив код близько 2 годин, а може і більше.

Це нормально.

EpsilonMordecai написав:

Не певен і зараз, чи повністю вірно функціонує программа.

Додайте тести, це дуже сильно допомагає зменшити непевність.

EpsilonMordecai написав:

Чи може це бути пов'язано з тим, як я написав код?

Напевно із цим теж.

EpsilonMordecai написав:

Чи може є інші проблеми?

Так. Проблема, яку ви описали - непевність - психологічна, а не технічна.

EpsilonMordecai написав:

Посилання на гугл диск

Стандарт спільноти - публічний репозиторій, найчастіше на GitHub. До речі, запровадження СКВ теж дуже сильно допомагає зменшити непевність, бо зникає страх щось зіпсувати.

Наскільки я зрозумів, ви хочете перегляд коду (code review).
Ну ок, поїхали. main.cpp:

std::string input = {45, 67, 101, -23};

означає, що ви створюєте стрічку з 4 символів з кодами 45, 67, 101 і 233 ("-Ceй", якщо кодування Win-1251). Звісно, ви її одразу перезаписуєте в getline, але такий код заплутує читача (у першу чергу - вас).
Далі ви в циклі читаєте стрічку і викликаєте методи lang_prc. Причому умова виходу - порожня стрічка, а ви і при порожній стрічці викликаєте ті методи. Ні, може, воно і нормально працює, але логічніше було б зробити цикл з виходом усередині і переривати його за допомогою break після введення.

Подякували: EpsilonMordecai1

4

Re: Оцініть, будь ласка, код на С++. Дайте поради

Чесно кажучи, не дуже зрозуміло, що потрібно на вхід програми, та що вона має видати. Складається з послідовностей чого (чисел)? Можна приклад цього виразу? На скільки я зрозумів із ТЗ - на вхід іде послідовність, ну наприклад із вашого коду, "45, 67, 101, -23". Нехай десь в цій послідовності зустрінеться "*", який непевне десь має бути "45, 67, *, 101, -23". Отже на виході ми отримаємо, що? Послідовність виду - "45, 67101, -23" (множення(конкатенація) виразу 67, *, 101 - я зрозумів так  67101)? Конкретизуйте.

5

Re: Оцініть, будь ласка, код на С++. Дайте поради

Перепрошую, попередній коментар я надіслав із затримкою в 3 години, світло вимикали.
Ідемо далі. Prc.h. Що взагалі означає "prc"? Китайська Народна Республіка англійською? Назви мають бути змістовні.
Простір імен Lang. Мабуть, від language. Щось, пов'язане із мовою. Ну добре, припустимо.

        enum class TOKEN_ID
        {
            TERM_PROD,
            MULT,
            BOB = NULL
        };

Значеннями enum-а можуть бути лише цілі числа. NULL - посилання на адресу 0 - буде перетворене на число 0, і, відповідно, BOB і TERM_PROD означають одне й те саме. Вам саме так треба?

У класів є конструктори за замовчуванням. Загалом, я б сказав, це не дуже добре. Звісно, є безліч ситуацій, де такі конструктори потрібні, але наявність функції init без серйозного приводу - це вже не дуже добре. Скажімо, в тому ж main могло б бути

    while (...)
    {
        getline(std::cin, input);
        Lang::Prc lang_prc(input);
        std::cout << ">>>\t" << lang_prc.get_last_out() << "\n";
    }

Погодьтеся, дещо елегантніше - об'єкт створюється лише для того, щоб видати результат. Хоча, звісно, якщо конструктор кидає виключні ситуації, краще з init. А от нащо Token-у конструктор за замовчуванням - я не зрозумів. А ще Token та TOKEN_ID можуть бути приватними, їх ніби ніхто поза класом Prc бачити не має.

Так, переходимо до найцікавішого. prc.cpp.
Для початку - нащо вам потрібен id? Ви його ніде не використовуєте.
Списками ініціалізації ви теж чомусь не користуєтеся. Гм.
*(tokens.cend() - 1); має значно кращу назву - tokens.back(), а *tokens.cbegin() - це tokens.front().
Lexer. Тут у вас є кілька замикань, які оцінюють параметр (ще й дублюють стандартні функції з cctype) і змінюють захоплені змінні. Це погано і значно ускладнює зневадження. Причому ви ж самі вимушені писати порожнє тіло гілки if через це. Порівняйте ваш код

    auto is_string = [&state](char symbol)
    {
        if ((symbol >= 'a' && symbol <= 'z') || (symbol >= 'A' && symbol <= 'Z'))
        {
            state = LEXER_STATE::STRING;
            return true;
        }
        return false;
    };
        if (i != -1 && is_string(exp[i])) {}

і ось такий:

        if (i!=-1 && isalpha(exp[i])) {
            state = LEXER_STATE::STRING;
        }

Правда, у другому мороки значно менше?
Так, я бачу, ви хочете одним is_correct робити одночасно купу справ. Але ви тільки себе плутаєте.

Подякували: EpsilonMordecai1

6

Re: Оцініть, будь ласка, код на С++. Дайте поради

lucas-kane написав:

Чесно кажучи, не дуже зрозуміло, що потрібно на вхід програми, та що вона має видати. Складається з послідовностей чого (чисел)? Можна приклад цього виразу? На скільки я зрозумів із ТЗ - на вхід іде послідовність, ну наприклад із вашого коду, "45, 67, 101, -23". Нехай десь в цій послідовності зустрінеться "*", який непевне десь має бути "45, 67, *, 101, -23". Отже на виході ми отримаємо, що? Послідовність виду - "45, 67101, -23" (множення(конкатенація) виразу 67, *, 101 - я зрозумів так  67101)? Конкретизуйте.

Наскільки я зрозумів, воно лише викидає пробіли і зірочки зі стрічки, але дуже хитромудро, через лямбди і скінчений автомат.

Подякували: EpsilonMordecai1

7

Re: Оцініть, будь ласка, код на С++. Дайте поради

Дякую усім!
Використовуючи ваші рекомендації, отримав наступне: https://github.com/Epsilon-Mordecai/StrPrc. Дякую!
Нібито маю лише невпевненість, яку можна подолати практикою. Фактичних питань ніби немає.

8 Востаннє редагувалося koala (16.01.2023 07:09:39)

Re: Оцініть, будь ласка, код на С++. Дайте поради

            if (state != LEXER_STATE::STRING)
            {
                state = LEXER_STATE::STRING;
            }

Це просто

state = LEXER_STATE::STRING;

Після виконання цього коду state у будь-якому разі стане LEXER_STATE::STRING, то нащо код ускладнювати?

А тут

if (i != 0 && !isalpha(exp[i - 1]) || i == 0)

можна викинути i != 0, тому що випадок i == 0 уже покритий другою умовою:

if (i == 0 || !isalpha(exp[i - 1]))

Але тепер ви ж узагалі не використовуєте значення state, тому можна його повністю видалити.

Подякували: EpsilonMordecai1

9 Востаннє редагувалося koala (16.01.2023 07:38:29)

Re: Оцініть, будь ласка, код на С++. Дайте поради

Ну і зрештою - я ж правий, цей код перетворює стрічку виду

c1*c2*c3*...*cn

де ci - символ алфавіту і є довільна кількість пробілів, на

c1c2c3...cn

правильно?

std::string convert(const std::string& source) {
  std::string result;
  bool is_prev_alpha = false;
  for(char c: source) {
    if(isspace(c))
      continue;
    bool is_c_alpha = isalpha(source[pos]);
    if(!is_c_alpha && c!='*')
        throw ...;
    if(is_prev_alpha == is_c_alpha)
        throw ...;
    is_prev_alpha = is_c_alpha;
    if(is_c_alpha)
        result += c;
  }
  return result;
}

Якось так.

Подякували: EpsilonMordecai1

10

Re: Оцініть, будь ласка, код на С++. Дайте поради

Так, Ви праві, программа саме це і повинна робити.
Я і справді дуже складно написав вирішення цієї задачі. От чому мені так сладно писати навіть такі прості программи. У мене є питання. Як можна вирішити цю проблему? Думав, може вивчати алгоритми і пристосовувати мозок до такого типу мислення. Інші кажуть, що алгоритми не дуже потрібні.
Щодо программи. Я скоротив її ще, за Вашими рекомендаціями, але останній варіант, що ви запропонували мені не дуже подобається, бо я планую розширювати функціонал программи. Мені здається, що легше це буде робити з тієї точки, що в мене є зараз на репозиторії.

Дякую Вам!

11

Re: Оцініть, будь ласка, код на С++. Дайте поради

Більшість відповідей - тут. Якщо щось незрозуміло, уточнюйте.
Про алгоритми: все, що ви робите - це перекладаєте алгоритми з людської мови на комп'ютерну. Звісно, вам треба знати основні алгоритми; і якщо без алгоритму побудови найкоротшого шляху в лабіринті ви ще якось проживете, то алгоритм бінарного пошуку розуміти треба.

Подякували: EpsilonMordecai1