做关于兔子群的练习,撞墙了

Doing a exercise about bunny colony, hit a wall

我正在做这个列表中的最后一个练习(它叫做毕业):http://www.cplusplus.com/forum/articles/12974/但是有一个主要问题。我写的代码可以运行,但有时会崩溃(在删除半个兔子之后),有时在程序第一次删除半个兔子之后,有时在 10 个这样的循环之后,请注意我还没有实现很多,因为我想修复这个错误,当然是在你的帮助下。我也知道这不是代码审查,但一些关于风格和改进的小技巧也会很好。所以这是我到目前为止写的代码:

Main.cpp:

include bunnyList.h
include windows.h

using namespace std;

int main(){
    srand(time(NULL));
    bunnyList Colony;
    int turns = 0;
    Colony.setUp();
    while(Colony.getColonySize() > 0){
        //New turn
        Colony.increaseAgeAndKill();
        Colony.breedBunnies();

        std::cout << "Turn: "<< turns << ". Colony size: " << Colony.getColonySize() << std::endl;
        //Get rid of these food eaters
        if(Colony.getColonySize() > 1000){
        std::cout << "500 bunnies died!" << std::endl;
        Colony.killHalfBunnies();
        }
    Sleep(100);
    turns++;;
    }
}

bunnyList.h:

#ifndef BUNNYLIST_H
#define BUNNYLIST_H

#include <stdlib.h>
#include "node.h"
#include <time.h>
#include <iostream>
#include <string>

const int numOfNames = 4;
const int numOfColors = 4;
const int bunniesIni = 5;
const std::string colors[numOfColors] = {"Black", "White", "Brown", "Spotted"};
const std::string maleNames[numOfNames] = {"Joe", "Rafael", "Buby", "Messi"};
const std::string femaleNames[numOfNames] = {"Reichel", "Agnesa", "Mr Flufy", "Flower"};

class bunnyList{
private:
    node *head;
    int noOfBunnies;
    node *current, *prev;
public:

    bunnyList();

    void newBunny(std::string);
    void killHalfBunnies();
    void increaseAgeAndKill();
    void deleteNode();
    void breedBunnies();
    void setUp();

    int getRandomNumber(int) const;
    std::string getRandomColor();
    std::string getRandomName(bool);
    bool isMaleRandom();

    int getColonySize() const;
};

#endif

bunnyList.cpp:

#include "bunnyList.h"

bunnyList::bunnyList(){
        noOfBunnies = 0;
}

void bunnyList::setUp(){
        std::string temp = "";
        head = NULL;
        for(int i = 0; i <= bunniesIni; i++){
            newBunny(temp);
        }
}

void bunnyList::killHalfBunnies(){
    prev = head;
    current = head;

    while(noOfBunnies > 500){
        if(getRandomNumber(2) == 1){
            deleteNode();
            continue;
        } else if(current == NULL){
            current = head;
            prev = head;
       } else {
            prev = current;
            current = current->next;
            continue;
        }

    }
}

void bunnyList::newBunny(std::string color){
    node *bunny = new node();
    node *temp = head;

    if(color == ""){
        bunny->color = getRandomColor();
    } else {
        bunny->color = color;
    }

    bunny->isMale = isMaleRandom();
    bunny->name = getRandomName(bunny->isMale);
    bunny->age = 0;
    bunny->next = NULL;
    bunny->isBreedable = 0;

    if(head == NULL){
        head = bunny;
        return;
    }

    while(temp->next != NULL){
        temp = temp->next;
    }

    temp->next = bunny;
    noOfBunnies++;
}

void bunnyList::increaseAgeAndKill(){
    current = head;
    prev = head;
    while(current != NULL){
        current->age++;
        //Check if bunny can breed
        if(current->age > 2){
            current->isBreedable = 1;
        }
        //Check if its time to die :/
        if(current->age > 10){
            deleteNode();
        }
        prev = current;
        current = current->next;
    }
    current = head;
    prev = head;
}

void bunnyList::breedBunnies(){
    node *temp = head;
    bool oneMale = 0;
    int femaleCount = 0;
    //Check if there is at least one breedable male
    while(temp!=NULL){
        if(temp->isMale && temp->isBreedable){
            oneMale = 1;
            break;
        }
        temp = temp->next;
    }
    //For every female bunny over 2 years old a new bunny is born
    temp = head;
    if(oneMale){
    while(temp != NULL){
            if(temp->isMale == 0 && temp->isBreedable){
                newBunny(temp->color);
            }
        temp = temp->next;
        }
    }
}

void bunnyList::deleteNode(){
    if(current==head){
        head = current->next;
        prev = head;
        delete current;
        current = head;
        noOfBunnies--;
    } else if(current->next==NULL){
        delete current;
        prev->next = NULL;
        prev = head;
        current = head;
        noOfBunnies--;
    } else {
        prev->next = current->next;
        current->next = NULL;
        delete current;
        current = prev->next;
        noOfBunnies--;
    }
}

std::string bunnyList::getRandomName(bool isMale){
    int r = getRandomNumber(numOfNames - 1);
    if(isMale)
    return maleNames[r];
    return femaleNames[r];
}

std::string bunnyList::getRandomColor(){
    int r = getRandomNumber(numOfColors - 1);
    return colors[r];
}

bool bunnyList::isMaleRandom(){
    if(getRandomNumber(2) == 1) {return true;}
    return false;
}

int bunnyList::getRandomNumber(int limit) const{
    return rand() % limit + 1;
}

int bunnyList::getColonySize() const{
    return noOfBunnies;
}

node.h:

#ifndef NODE_H_INCLUDED
#define NODE_H_INCLUDED

#include <string>

class node {
    friend class bunnyList;
private:
    std::string name;
    int age;
    std::string color;
    bool isMale;
    node *next;
    bool isBreedable;
public:
};

#endif // NODE_H_INCLUDED

感谢您的帮助。

既然你要求评论...

永远不要写 using namespace std。绝不。就在今天早上,在 SO 上提出了一个问题,手头问题的原因是那条臭名昭著的线路。我想知道是谁以及为什么建议这是一个好方法 - 某处应该有一本书。如果我按照我的方式行事,它的作者将被谴责永远从每个文件中手动删除这一行。

即使不阅读代码中的一行,仅通过解释,我也知道问题很可能(100% 可能)与内存管理有关。您正在释放未正确分配的内存,您正在释放同一内存两次,或者您正在释放根本未分配的内存,或者您正在释放内存后访问内存。查看您的 delete 并检查它们。

关于风格。您的代码基本上是业务逻辑感知列表的实现。通常,这不是一个好的做法。实现一个泛型列表,支持增删改等泛型列表操作,比在这个泛型列表之上实现你的业务逻辑要好得多。

不要在列表中使用 current。相反,在您的删除函数中传递一个要删除的节点。

最后,运行 您在调试器中的程序并查看您要删除的变量。

编辑 在评论中回答问题。

这就是我所说的业务逻辑分离的意思。有一个通用的数据结构,称为列表。它可以是任何东西的列表,兔子或 space 火箭,没关系 - 但它仍然支持基本的列表操作。显然,最重要的两个是插入和删除,但它不是泛型列表的唯一操作。您可以在列表(数据结构)上阅读维基百科以了解一般想法,并在实施中查看 std::list。现在,您有了列表的特定用例,一个兔子列表。对于该特定用例,您将在通用列表之上添加功能。为了进一步澄清,从列表中删除一个项目是通用列表支持的。但是 'killing a rabit' 当这个可怜的生物老了 10 岁时,这是一种商业逻辑。它包含遍历兔子列表(由通用列表提供)、检查年龄并决定消除该生物(业务逻辑级别)以及删除元素(通用列表)。如果使用 std::list 编写此代码,则大致如下:

std::list<Bunny> bunnies;
for (auto bunny = bunnies.cbegin(), end = bunnies.cend(); bunny != end; ++bunny) {
    if (bunny->age() > 10)
        bunny = bunnies.erase(bunny);
}

我也遇到了崩溃。它在 deleteNode 中崩溃,试图在 current 为 NULL 时引用 current->next

这是从 bunnyList::killHalfBunnies 调用的,在这段代码中,这是我发现的问题:

    if (getRandomNumber(2) == 1){
        deleteNode();
        continue;
    }
    else if (current == NULL){
        current = head;
        prev = head;
    }

问题是您调用 deleteNode假设 current 不为 NULL, 检查之前current 实际上不是 NULL。我重新排列了 if,如下所示,我不再遇到崩溃:

    if (current == NULL)
    {
        current = head;
        prev = head;
    }
    else if (getRandomNumber(2) == 1)
    {
        deleteNode();
        continue;
    }

deleteNode里面放一个检查也是明智的,这样如果在current为NULL时调用它,它可以处理它。可能通过抛出异常或以其他方式警告您。

由于您还询问了风格:deleteNode(和其他地方)中的评论会让一切变得更加清晰!

我假设这是一个 class 作业,这就是为什么你没有使用 std::list