C ++ эта функция будет течь? - PullRequest
Новая
       8

C ++ эта функция будет течь?

2 голосов
/ 21 января 2010

Я начал писать простую консольную игру Yahtzee для практики. У меня просто вопрос относительно того, будет ли эта функция пропускать память. Функция броска вызывается каждый раз, когда кубики нужно перекатывать.

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

Основная функция 

int *kast = NULL;           //rolled dice
int *keep_dice = NULL;    //which dice to re-roll or keep

kast = roll(kast, keep_dice);
delete[] kast;

и вот функция 

int *roll(int *dice, int *keep) {

    srand((unsigned)time(0));
    int *arr = new int[DICE];
    if(!dice)
    {
        for(int i=0;i<DICE;i++)
        {

            arr[i] = (rand()%6)+1;
            cout << arr[i] << " ";
        }
    }
    else
    {
        for(int i=0;i<DICE;i++)
        {
            if(!keep[i])
            {
                dice[i] = (rand()%6)+1;
                cout << "Change ";
            }
            else
            {
                keep[i] = 0;
                cout << "Keep ";
            }
        }
        cout << endl;
        delete[] arr;
        arr = NULL;
        arr = dice;

    }
    return arr;
}

Ответы [ 4 ]

12 голосов
/ 21 января 2010

Да, это может протечь. Например, использование cout может вызвать исключение, и если это произойдет, ваш delete никогда не будет вызван.

Вместо того, чтобы выделять динамический массив самостоятельно, вы можете рассмотреть возможность возврата std::vector. Более того, превратите вашу функцию в правильный алгоритм, который берет итератор (в данном случае back_insert_iterator) и записывает свои выходные данные.

Редактировать: Глядя на это более внимательно, я чувствую себя обязанным отметить, что мне действительно не нравится базовая структура этого кода. У вас есть одна функция, которая действительно выполняет две разные вещи. У вас также есть пара массивов, которые вы зависите от параллельной адресации. Я бы реструктурировал его в две отдельные функции: roll и re_roll. Я бы реструктурировал данные в виде массива структур: 

struct die_roll { 
    int value;
    bool keep;

    die_roll() : value(0), keep(true) {}
};

Чтобы выполнить начальный бросок, вы передаете вектор (или массив, если вы действительно настаиваете) из них в функцию roll, которая заполняет начальные значения. Чтобы выполнить переброс, вы передаете вектор в re-roll, который перебрасывает, чтобы получить новое значение для любого die_roll, для которого keep член был установлен в false.

4 голосов
/ 21 января 2010

То, как вы выделяете память, сбивает с толку: память, выделенная внутри функции, должна освобождаться кодом вне функции.

Почему бы не переписать это примерно так: 

int *kast = new int[DICE];           //rolled dice
bool *keep_dice = new bool[DICE];    //which dice to re-roll or keep
for (int i = 0; i < DICE; ++i)
    keep_dice[i] = false;

roll(kast, keep_dice);

delete[] kast;
delete[] keep_dice;

Это хорошо соответствует вашим new с и delete с. Что касается функции: поскольку мы устанавливаем keep_dice all на false, ни один из аргументов не является NULL, и он всегда изменяет dice вместо возврата нового массива, он упрощается до: 

void roll(int *dice, int *keep) {
    for(int i=0;i<DICE;i++)
    {
        if(keep[i])
        {
            keep[i] = false;
            cout << "Keep ";
        }
        else
        {
            dice[i] = (rand()%6)+1;
            cout << "Change ";
        }
    }
    cout << endl;
}

Кроме того, вы должны переместить вызов srand в начало вашей программы. Повторный посев крайне вреден для случайности.

4 голосов
/ 21 января 2010

Используйте (выделенный стеком) std::vector вместо массива и передайте ссылку на него функции. Таким образом, вы будете уверены, что он не протекает.

0 голосов
/ 21 января 2010

Я бы посоветовал взять тайм-аут на покупку / заимствование и прочитать Скотт Мейерс Эффективный C ++, 3-е издание. Вы сэкономите месяцы боли, пытаясь стать продуктивным программистом C ++. И я говорю из личного горького опыта.

...