Моделирование популяции Doodlebug и ant

Я ищу обзор одного из моих домашних заданий на этот семестр.

Эта домашняя работа уже была представлена ​​и оценена, и мой финал уже представлен, поэтому в моем запросе на отзыв нет обмана или конфликта интересов!

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

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

Включает в себя

#include "stdafx.h"
#include <iostream>
#include <ctime>
#include <vector>

using namespace std;

** Обратите внимание, что координаты struct просто содержат целое число xCoordinate и целое число yCoordinate

Основной

int main()

{
    //Create environment object containing 
    environment antDoodlebugSimulation;

    antDoodlebugSimulation.InitializeSimulation();

    return 0;
}

Окружающая среда

class environment
{
    //friends of environment
    friend class organism;
    friend class doodlebug;
    friend class ant;

private:
    organism * environmentBoard[20][20];
    void CreateStartPopulation();
    int GenerateRandomStartingLocations(int min, int max);
    void OutputCurrentEnvironment();
    void DoodlebugsAct();
    void AntsAct();
    void ResetCritterTimeStep();

public:
    //constructor
    environment();

    //deconstructor
    ~environment();

    //public member functions
    void InitializeSimulation();

};

environment::environment()
{
    //Initialize environmentBoard array for entirely empty...all nullptr
    for (int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
            this->environmentBoard[xCounter][yCounter] = nullptr;

    //Add ants and doodlebugs into game environment
    CreateStartPopulation();
}

void environment::OutputCurrentEnvironment()
{
    //Outputs current environment to the screen
    for (int yCounter = 0; yCounter < 20; yCounter++)
    {
        for (int xCounter = 0; xCounter < 20; xCounter++)
            if (this->environmentBoard[xCounter][yCounter] == nullptr)
                cout << '-';
            else
                cout << *environmentBoard[xCounter][yCounter];

        cout << endl;
    }
}

void environment::CreateStartPopulation()
{
    //Populates game board for start of simulation
    coordinates newLocation;

    //Fill 5 doodlebugs
    for (int doodleCounter = 0; doodleCounter < 5; doodleCounter++)
    {
        newLocation.xCoordinate = GenerateRandomStartingLocations(0, 19);
        newLocation.yCoordinate = GenerateRandomStartingLocations(0, 19);

        //Incase something already placed
        while (this->environmentBoard[newLocation.xCoordinate][newLocation.yCoordinate] != nullptr)
        {
            newLocation.xCoordinate = GenerateRandomStartingLocations(0, 19);
            newLocation.yCoordinate = GenerateRandomStartingLocations(0, 19);
        }

        this->environmentBoard[newLocation.xCoordinate][newLocation.yCoordinate] = new doodlebug(newLocation.xCoordinate, newLocation.yCoordinate, this);
    }
    for (int antCounter = 0; antCounter < 100; antCounter++)
    {

        newLocation.xCoordinate = GenerateRandomStartingLocations(0, 19);
        newLocation.yCoordinate = GenerateRandomStartingLocations(0, 19);

        //Incase something already placed
        while (this->environmentBoard[newLocation.xCoordinate][newLocation.yCoordinate] != nullptr)
        {
            newLocation.xCoordinate = GenerateRandomStartingLocations(0, 19);
            newLocation.yCoordinate = GenerateRandomStartingLocations(0, 19);
        }

        this->environmentBoard[newLocation.xCoordinate][newLocation.yCoordinate] = new ant(newLocation.xCoordinate, newLocation.yCoordinate, this);
    }
}

int environment::GenerateRandomStartingLocations(int min, int max)
{
    static bool firstIteration = true;
    if (firstIteration)
    {
        //Seed once
        srand(time(NULL));
        firstIteration = false;
    }
    //Generate random number between range
    return rand() % (max - min + 1) + min;
}

void environment::AntsAct()
{

    //Iterate over environment
    for(int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            if (this->environmentBoard[xCounter][yCounter] != nullptr)
            {
                if (this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'o' && !(this->environmentBoard[xCounter][yCounter]->GetMoveAlternatorStatus()))
                {   
                    //Toggle moveAlternator
                    this->environmentBoard[xCounter][yCounter]->ToggleMoveAlternator();

                    //Move ant....Move function must return coordinate otherwise we attempt to increment/breed null pointer after movement
                    this->environmentBoard[xCounter][yCounter]->Move();
                }
            }
        }

    //Iterate over Ants again and breed
    for(int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            if (this->environmentBoard[xCounter][yCounter] != nullptr)
            {
                if (this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'o')
                {
                    //Increment Ant Breeding cycle
                    this->environmentBoard[xCounter][yCounter]->IncrementBreedingCycle();

                    //Check if ready to breed
                    if (this->environmentBoard[xCounter][yCounter]->TimeToBreed())
                    {
                        this->environmentBoard[xCounter][yCounter]->Breed();
                    }
                }
            }
        }
}

void environment::DoodlebugsAct()
{
    //Move and eat if ant is found
    for (int yCounter = 0; yCounter < 20; yCounter++)
    {
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            //Not null pointer...a doodlebug....and has not already moved
            if (this->environmentBoard[xCounter][yCounter] != nullptr && this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'X' && !this->environmentBoard[xCounter][yCounter]->GetMoveAlternatorStatus())
            {
                this->environmentBoard[xCounter][yCounter]->ToggleMoveAlternator();
                this->environmentBoard[xCounter][yCounter]->Move();
            }
        }
    }
    //Breed and then check death cycle
    for (int yCounter = 0; yCounter < 20; yCounter++)
    {
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            if (this->environmentBoard[xCounter][yCounter] != nullptr && this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'X')
            {
                //Increment breeding step
                this->environmentBoard[xCounter][yCounter]->IncrementBreedingCycle();

                //Check if ready to breed
                if (this->environmentBoard[xCounter][yCounter]->TimeToBreed())
                    this->environmentBoard[xCounter][yCounter]->Breed();

                //Check if dying
                if (this->environmentBoard[xCounter][yCounter]->Death())
                    this->environmentBoard[xCounter][yCounter]->Die();
            }
        }
    }
}

void environment::ResetCritterTimeStep()
{
    //Reset all critter time steps at the end of iteration
    for (int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
            if (this->environmentBoard[xCounter][yCounter] != nullptr && (this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'o' || this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'X'))
                this->environmentBoard[xCounter][yCounter]->ToggleMoveAlternator();
}

void environment::InitializeSimulation()
{
    int numOfants, numOfDoodleBugs;

    OutputCurrentEnvironment();
    cout << "Simulation initialized starting with 5 doodle bugs and 100 ants.  Please begin simulation by pressing the enter key\n";

    while(cin.get() == '\n')
    {
        //Manage simulation
        numOfants = 0;
        numOfDoodleBugs = 0;

        //Call doodlebugs to act
        DoodlebugsAct();

        //Call ants to act
        AntsAct();

        //Reset all critter time steps
        ResetCritterTimeStep();

        //output graphical environment status
        OutputCurrentEnvironment();

        //For easy visual of fluctuations between predator and prey populations
        for (int yCounter = 0; yCounter < 20; yCounter++)
            for (int xCounter = 0; xCounter < 20; xCounter++)
                if (this->environmentBoard[xCounter][yCounter] != nullptr)
                    if (this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'o')
                        numOfants++;
                    else
                        numOfDoodleBugs++;

        cout << "Ants: " << numOfants << endl;
        cout << "Doodlebugs: " << numOfDoodleBugs << endl;

        cout << "To continue simulation please press enter; otherwise enter -1: ";
    }
}

environment::~environment()
{
    //Clean up memory for object and repoint all dangling pointers
    for(int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            if (this->environmentBoard[xCounter][yCounter] != nullptr)
            {
                delete this->environmentBoard[xCounter][yCounter];
                this->environmentBoard[xCounter][yCounter] = nullptr;
            }
        }
}

Организм

class organism
{
    //Organism parent class
    private:
        bool moveAlternator = false;

    protected:
        environment * environmentPointer;   
        int breedingCycle = 0;
        char identifierTag;
        coordinates location;

    public:
        //Constructor
        organism() {location.xCoordinate = 0; location.yCoordinate = 0;};
        //deconstructor
        virtual ~organism() {};
        char GetIdentifierTag() const { return this->identifierTag; };
        void IncrementBreedingCycle() { breedingCycle += 1; };
        coordinates GetLocation() const { return this->location; };
        virtual void Move() = 0;
        void RandomDirectionalMovement(int direction);
        virtual bool TimeToBreed() = 0;
        void Die();
        virtual bool Death() =0;
        void Breed();
        void ResetBreedingCycle() { this->breedingCycle = 0; };
        void ToggleMoveAlternator();
        bool GetMoveAlternatorStatus() const { return moveAlternator; };
        void GoNorth() { this->location.yCoordinate -= 1; };
        void GoEast() { this->location.xCoordinate += 1; };
        void GoSouth() { this->location.yCoordinate += 1; };
        void GoWest() { this->location.xCoordinate -= 1; };

        friend ostream &operator<<(ostream &output, const organism &currentOrg) { output << currentOrg.identifierTag; return output; };
};

void organism::ToggleMoveAlternator()
{
    if (this->moveAlternator == false)
        this->moveAlternator = true;
    else
        this->moveAlternator = false;
}

void organism::Breed()
{
    //Breeds new 
    //Generate direction to breed in...if empty..breed
    int newDirection = this->environmentPointer->GenerateRandomStartingLocations(1, 4);
    bool breedSuccesful = false;

    //North
    if (newDirection == 1)
    {
        if (this->location.yCoordinate - 1 > -1 && this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1] == nullptr)
        {
            if (this->GetIdentifierTag() == 'o')//Breed new ant
                this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1] = new ant(this->location.xCoordinate, this->location.yCoordinate - 1, this->environmentPointer);
            else if (this->GetIdentifierTag() == 'X')//Breed new doodlebug
                this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1] = new doodlebug(this->location.xCoordinate, this->location.yCoordinate - 1, this->environmentPointer);

            breedSuccesful = true;
        }
    }
    //East
    else if (newDirection == 2)
    {
        if (this->location.xCoordinate + 1 < 20 && this->environmentPointer->environmentBoard[this->location.xCoordinate + 1][this->location.yCoordinate] == nullptr)
        {

            if(this->GetIdentifierTag() == 'o')//Breed new ant
                this->environmentPointer->environmentBoard[this->location.xCoordinate+1][this->location.yCoordinate] = new ant(this->location.xCoordinate+1, this->location.yCoordinate, this->environmentPointer);
            else if (this->GetIdentifierTag() == 'X')//Breed new doodlebug
                this->environmentPointer->environmentBoard[this->location.xCoordinate + 1][this->location.yCoordinate] = new doodlebug(this->location.xCoordinate + 1, this->location.yCoordinate, this->environmentPointer);

            breedSuccesful = true;
        }
    }
    //South
    else if (newDirection == 3)
    {
        if (this->location.yCoordinate + 1 < 20 && this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate + 1] == nullptr)
        {
            if (this->GetIdentifierTag() == 'o')//Breed new ant
                this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate+1] = new ant(this->location.xCoordinate, this->location.yCoordinate+1, this->environmentPointer);
            else if (this->GetIdentifierTag() == 'X')//Breed new doodlebug
                this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate+1] = new doodlebug(this->location.xCoordinate, this->location.yCoordinate+1, this->environmentPointer);

            breedSuccesful = true;
        }
    }
    //West
    else if (newDirection == 4)
    {
        if (this->location.xCoordinate - 1 > -1 && this->environmentPointer->environmentBoard[this->location.xCoordinate - 1][this->location.yCoordinate] == nullptr)
        {
            if (this->GetIdentifierTag() == 'o')//Breed new ant
                this->environmentPointer->environmentBoard[this->location.xCoordinate-1][this->location.yCoordinate] = new ant(this->location.xCoordinate-1, this->location.yCoordinate, this->environmentPointer);
            else if (this->GetIdentifierTag() == 'X')//Breed new doodlebug
                this->environmentPointer->environmentBoard[this->location.xCoordinate-1][this->location.yCoordinate] = new doodlebug(this->location.xCoordinate-1, this->location.yCoordinate, this->environmentPointer);

            breedSuccesful = true;
        }
    }

    if (breedSuccesful)
        this->ResetBreedingCycle();
}
void organism::RandomDirectionalMovement(int direction)
{
    //North
    if (direction == 1)
    {
        if (this->location.yCoordinate - 1 > -1 && this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1] == nullptr)
        {
            //Empty space found and valid location within stated environment
            //Point new location to current organism object
            this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1] = this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate];

            //repoint old location to null
            this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate] = nullptr;

            //update ant location
            this->GoNorth();
        }
    }
    //East
    else if (direction == 2)
    {
        if (this->location.xCoordinate + 1 < 20 && this->environmentPointer->environmentBoard[this->location.xCoordinate + 1][this->location.yCoordinate] == nullptr)
        {
            //Empty space found and valid location with stated environment
            //Point new location to current ant object
            this->environmentPointer->environmentBoard[this->location.xCoordinate + 1][this->location.yCoordinate] = this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate];

            //Repoint old location to null
            this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate] = nullptr;

            //update ant location
            this->GoEast();
        }
    }
    //South
    else if (direction == 3)
    {
        if (this->location.yCoordinate + 1 < 20 && this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate + 1] == nullptr)
        {
            //Empty space found and valid location with stated environment
            //Point new location to current ant object
            this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate + 1] = this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate];

            //Repoint old location to null
            this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate] = nullptr;

            //update organism location
            this->GoSouth();
        }
    }
    //West
    else if (direction == 4)
    {
        if (this->location.xCoordinate - 1 > -1 && this->environmentPointer->environmentBoard[this->location.xCoordinate - 1][this->location.yCoordinate] == nullptr)
        {
            //Empty space found and valid location with stated environment
            //Point new location to current organism object
            this->environmentPointer->environmentBoard[this->location.xCoordinate - 1][this->location.yCoordinate] = this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate];

            //Repoint old location to null
            this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate] = nullptr;

            //update organism location
            this->GoWest();
        }
    }
}

void organism::Die()
{
    //repoint location in environment to null
    this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate] = nullptr;

    //delete
    delete this;

Doodlebug

class doodlebug : public organism
{
    //Doodlebug child class

    private:
        int starveCounter = 0;

    public:
        //Constructor
        doodlebug(int xLocation, int yLocation, environment * worldPointer) { identifierTag = 'X'; location.xCoordinate = xLocation; location.yCoordinate = yLocation; environmentPointer = worldPointer; };
        //Deconstructor
        ~doodlebug() {};

        bool TimeToBreed();
        void Move();
        coordinates SearchForAnts();
        void IncrementStarveCounter() { this->starveCounter += 1; };
        void ResetStarveCounter() { this->starveCounter = 0; };
        bool Death() { if (this->starveCounter == 3) return true; else return false; };

};

coordinates doodlebug::SearchForAnts()
{
    coordinates antFound;
    antFound.xCoordinate = -1;
    antFound.yCoordinate = -1;

    //store checked direction
    int directionCheck;
    vector<int> storeDirections;

    directionCheck = this->environmentPointer->GenerateRandomStartingLocations(1, 4);

    //randomize first search direction and store already searched directions
    while (storeDirections.size() < 4 && antFound.xCoordinate == -1 && antFound.yCoordinate == -1)
    {
        //Search North for ant...verify not off the board...verify not null pointer...check if ant
        if (directionCheck == 1 && this->location.yCoordinate - 1 > -1 && this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1] != nullptr)
        {
            if (this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate - 1]->GetIdentifierTag() == 'o')
            {
                antFound.xCoordinate = this->location.xCoordinate;
                antFound.yCoordinate = this->location.yCoordinate - 1;
            }
        }
        //Search East for Ant....
        else if (directionCheck == 2 && this->location.xCoordinate + 1 < 20 && this->environmentPointer->environmentBoard[this->location.xCoordinate + 1][this->location.yCoordinate] != nullptr)
        {
            if (this->environmentPointer->environmentBoard[this->location.xCoordinate + 1][this->location.yCoordinate]->GetIdentifierTag() == 'o')
            {
                antFound.xCoordinate = this->location.xCoordinate + 1;
                antFound.yCoordinate = this->location.yCoordinate;
            }
        }
        //Search South for Ant....
        else if (directionCheck == 3 && this->location.yCoordinate + 1 < 20 && this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate + 1] != nullptr)
        {
            if (this->environmentPointer->environmentBoard[this->location.xCoordinate][this->location.yCoordinate + 1]->GetIdentifierTag() == 'o')
            {
                antFound.xCoordinate = this->location.xCoordinate;
                antFound.yCoordinate = this->location.yCoordinate + 1;
            }
        }
        //Search west for Ant...
        else if (directionCheck == 4 && this->location.xCoordinate - 1 > -1 && this->environmentPointer->environmentBoard[this->location.xCoordinate - 1][this->location.yCoordinate] != nullptr)
        {
            if (this->environmentPointer->environmentBoard[this->location.xCoordinate - 1][this->location.yCoordinate]->GetIdentifierTag() == 'o')
            {
                antFound.xCoordinate = this->location.xCoordinate - 1;
                antFound.yCoordinate = this->location.yCoordinate;
            }
        }

        //Store direction check to increase size of stored directions....guarantees 4
        storeDirections.push_back(directionCheck);
        if (directionCheck < 4)
        {
            directionCheck += 1;
        }
        else
            directionCheck = 1;
    }
    return antFound;
}

Ant

class ant : public organism
{
    //ant child class

    private:


    public:
        //Constructor
        ant(int xLocation, int yLocation, environment * thisWorld) { identifierTag = 'o'; location.xCoordinate = xLocation; location.yCoordinate = yLocation; environmentPointer = thisWorld; };

        //Deconstructor
        ~ant() {};

        void Move();
        bool TimeToBreed();
        bool Death() { return false; };

};
11 голосов | спросил StormsEdge 16 Maypm17 2017, 17:50:49

4 ответа


8

Я вижу некоторые вещи, которые могут помочь вам улучшить вашу программу.

Не злоупотреблять using namespace std

Ввод using namespace std в верхней части каждой программы плохая привычка , которую вам следует избегать.

Изолировать код, специфичный для платформы

Если у вас должен быть stdafx.h, рассмотрите его перенос таким образом, чтобы код был переносимым:

#ifdef WINDOWS
#include "stdafx.h"
#endif

Отдельный интерфейс от реализации

Интерфейс для класса обычно разделяется на файл .h, и реализация выполняется в .cpp. Преимущество в этом заключается в том, что он помогает программисту обеспечить хороший дизайн класса. Это приводит непосредственно к следующему предложению.

Переосмыслить иерархию классов

Понятно, что когда вы написали это, весь код был в одном файле. Это не обязательно неправильно, но это может привести к плохому дизайну классов, и это действительно так. Как сейчас, каждый класс должен знать обо всех других классах. Даже класс organism должен знать о ant и doodlebug! Это, безусловно, ошибка. Вместо этого я бы рекомендовал вам более четко выделить классы. Например, класс environment, возможно, должен знать о organism и, очевидно, ant и doodlebug должны знать о своем базовом типе organism, но это должно быть так. Некоторые из нижеприведенных предложений более четко излагают то, что требуется для этого.

Исправить ошибку

Функция SearchForAnts() в настоящее время не называется нигде, которая, если вы извините за каламбур, выглядит как ошибка для меня.

Не злоупотреблять friend

Класс environment имеет следующие три строки:

//friends of environment
friend class organism;
friend class doodlebug;
friend class ant;

Однако ни один из них не должен быть действительно нужен. Вместо этого было бы лучше просто переслать объявление класса organism и использовать для него только общедоступные интерфейсы.

Эффективно использовать наследование

environment имеет DoodlebugsAct() и AntsAct(), но это не имеет особого смысла. Если мы думаем о физическом аналоге, который он пытается имитировать, окружающая среда управляет действиями отдельных тварей или каждый твари воздействует в соответствии со своими собственными внутренними дисками и видом лишь небольшого подмножества окружающей среды, которое он может наблюдать? Я бы предположил, что это скорее будет последним, и это предполагает, что класс organism должен, вероятно, иметь функцию с именем act(), который будет реализован каждым производным классом и которому может передаваться вектор только соседних ячеек.

Предпочитают современные инициализаторы для конструкторов

Конструкторы могут использовать более современный стиль инициализации, а не старый стиль, который вы используете в настоящее время. Вместо этого:

ant(int xLocation, int yLocation, environment * thisWorld) { identifierTag = 'o'; location.xCoordinate = xLocation; location.yCoordinate = yLocation; environmentPointer = thisWorld; };

Напишите это:

ant(int xLocation, int yLocation, environment * thisWorld) : 
    identifierTag{'o'},
    location{xLocation, yLocation},
    environmentPointer{thisWorld}
{}

Обратите внимание, что это использует стиль инициализатора C ++ 11, который обеспечивает однозначный и последовательный синтаксис, а также более чистый код.

Рассмотрим использование лучшего генератора случайных чисел

В настоящее время вы используете

return rand() % (max - min + 1) + min;

С этим подходом существуют две проблемы. Во-первых, младшие разряды генератора случайных чисел не являются особенно случайными, поэтому и эти случайные числа не являются. На моей машине есть небольшое, но измеримое смещение к 0 с этим. Вторая проблема заключается в том, что он не является потокобезопасным, потому чтоrand хранит скрытое состояние. Лучшим решением, если ваш компилятор и библиотека его поддерживает, было бы использовать C ++ 11 `std :: uniform_int_distribution . Он выглядит сложным, но на самом деле он довольно прост в использовании.

Используйте более подходящую структуру данных

Текущий environment представляет собой сетку 20x20, содержащую либо указатели на организмы, либо nullptr. В большинстве случаев, в которых используется структура, было бы проще использовать в качестве простого линейного массива. Также возможно, чтобы каждый указатель был реальным указателем, а затем действия по умолчанию (возможно, не имеющие эффекта) могли быть созданы в базе organism класс.

Omit return 0

Когда программа C или C ++ достигает конца main, компилятор автоматически сгенерирует код для возврата 0, поэтому нет необходимости поставить return 0; явно в конце main.

Примечание: , когда я делаю это предложение, он почти всегда сопровождается одним из двух видов комментариев: «Я этого не знал». или «Это плохой совет!» Мое обоснование заключается в том, что безопасно и полезно полагаться на поведение компилятора, явно поддерживаемое стандартом. Для C, поскольку C99; см. раздел 5.1.2.2.3 стандарта ISO /IEC 9899: 1999:

  

[...] возврат от начального вызова к функции main эквивалентен вызову exit со значением, возвращаемым функцией main в качестве аргумента; достигнув }, который завершает функцию main возвращает значение 0.

Для C ++, начиная с первого стандарта в 1998 году; см. раздел ИСО /МЭК 14882: 1998 раздел 3.6.1:

  

Если элемент управления достигнет конца main без столкновения с оператором return, эффект будет выполняться при возврате 0;

Все версии обоих стандартов с тех пор (C99 и C ++ 98) поддерживают ту же идею. Мы полагаемся на автоматически сгенерированные функции-члены в C ++, и немногие люди пишут явные выражения return; в конце void. Причины против упущения, похоже, сводятся к " это выглядит странно ". Если, как и мне, вам любопытно обосновать изменение стандарта C прочитайте этот вопрос . Также обратите внимание, что в начале 1990-х годов это считалось «неряшливой практикой», поскольку в то время это было неопределенное поведение (хотя и широко поддерживаемое).

Поэтому я защищаю его; другие не согласны (часто яростно!) В любом случае, если вы столкнетесь с кодом, который его не выпускает, вы поймете, что он явно поддерживается стандартом, и вы узнаете, что это значит.

ответил Edward 16 Maypm17 2017, 23:00:29
9

Не используйте пространство имен std;

Это плохая привычка, которая может вызвать у вас проблемы. Возможно, вы захотите его найти.

Избегайте магических чисел

Например, размер вашей доски - 20. И по всему коду есть 20 подкрашенных. Что происходит, когда вы меняете размер доски и забываете одно из этих мест. Лучше создайте переменную члена bordSize класса environement и используйте ее во всем коде. Это делает его более читаемым.

То же самое касается таких вещей

//North
if (direction == 1)

Создайте перечисления типа

enum direction {
    NORTH,
    SOUTH,
    EAST,
    WEST
};

Затем вы можете сделать

if (direction == NORTH)

Использовать магические числа последовательно

У вас есть 2 магических числа в коде 19 и 20. Но подождите, они одинаковы. Разница в том, что вы GenerateRandomStartingLocations() включены. Опять же, нет причин, почему эта функция должна принимать аргументы вообще. создайте переменную-член и используйте ее внутри GenerateRandomStartingLocations()

Избегайте использования this-> внутри класса

this-> необходимо, только если вы столкнулись с именами. Всегда лучше просто не иметь коллизий имен, а не использовать this->

Использовать согласованные фигурные скобки

void environment::OutputCurrentEnvironment()
{
    //Outputs current environment to the screen
    for (int yCounter = 0; yCounter < 20; yCounter++)
    {
        for (int xCounter = 0; xCounter < 20; xCounter++)
            if (this->environmentBoard[xCounter][yCounter] == nullptr)
                cout << '-';
            else
                cout << *environmentBoard[xCounter][yCounter];

        cout << endl;
    }
}

Второй цикл для цикла не имеет браслета, и в нем он содержит однострочный оператор, который содержит более 4 строк. Это обязано вводить ошибки в долгосрочной перспективе. Также в таких простых конструкциях if вам лучше использовать тернарный оператор

void environment::OutputCurrentEnvironment()
{
    //Outputs current environment to the screen
    for (int yCounter = 0; yCounter < boardSize; yCounter++)
    {
        for (int xCounter = 0; xCounter < boardSize; xCounter++) 
        {
            std::cout << environmentBoard[xCounter][yCounter] == nullptr ? '-' : *environmentBoard[xCounter][yCounter] << "\n";
        }
    }
}

Избегайте промывки cout

std::endl очищает поток, который почти всегда не нужен, поэтому используйте простую новую строку «\ n»

Избегать бессмысленных комментариев

Вышеуказанный комментарий //Outputs current environment to the screen ничего не добавляет к коду. Так что просто опустите это.

Избегайте бесконечных линий

Чтение является ключевым. Всякий раз, когда вам нужно прокручивать влево или вправо, ваши линии слишком длинны. Большинство проектов с открытым исходным кодом идут на 80 символов, что вполне разумно.

При необходимости используйте конструкторы базового класса

Вы можете отложить конструктор производного класса к конструктору базового класса https://stackoverflow.com/questions/6923722/how-do- я с вызовом по-базового класса-конструктор

Упростить if /else с возвратом boolean

У вас есть код if condition return true else false Вы можете упростить это значение до return condition или return !condition

То же самое относится к назначениям

if (this->moveAlternator == false)
    this->moveAlternator = true;
else
    this->moveAlternator = false;

Является эквивалентным

moveAlternator = !moveAlternator;

Возможно, вы захотите использовать лямбда-функции

Много вашего кода невероятно повторяется. Например, эти строки появляются очень часто

for (int yCounter = 0; yCounter < boardSize; yCounter++)
{
    for (int xCounter = 0; xCounter < boardSize; xCounter++)
    {

Впоследствии, то, что вы делаете внутри кода, в основном мало и хорошо содержит. Здесь вы можете определить лямбда-функции и передать их в качестве аргументов функции, которая просто выполняет итерацию по циклам, а затем вызывает функцию https://stackoverflow.com/questions/3203305/написать-а-функцию, то-принимает-а-лямбда-выражение-в качестве аргумента-

https://stackoverflow.com/questions/8109571/lambda-as-function-parameter

ответил miscco 16 Maypm17 2017, 20:45:03
5

Вы, очевидно, долго и упорно работали по этому вопросу.

Когда класс наследуется от другого класса, конструктор базового класса называется импликацией конструктором для подкласса. Для инициализации переменных частного базового класса инициализация переменных в конструкторе базового класса. Чтобы изменить частные переменные базового класса базовый класс должен обеспечивать защищенные функции доступа. Например, Логическая переменная moveAlternator может иметь функцию-определитель, а также функцию gettor:

void SetMoveAltertor(bool NewValue)
{
    moveAlternator = NewValue;
}

или может быть функция переключения

void ToggleMoveAlternator()
{
    moveAlternator = (moveAlternator == true) ? false : true;
}

Использование этого указателя в C ++

В отличие от некоторых языков, таких как PHP this не требуется для ссылки на функции-члены или переменными-членами. поэтому такой код в среде :: AntsAct ()

    //Iterate over environment
    for(int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            if (this->environmentBoard[xCounter][yCounter] != nullptr)
            {
                if (this->environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'o' && !(this->environmentBoard[xCounter][yCounter]->GetMoveAlternatorStatus()))
                {   
                    //Toggle moveAlternator
                    this->environmentBoard[xCounter][yCounter]->ToggleMoveAlternator();

                    //Move ant....Move function must return coordinate otherwise we attempt to increment/breed null pointer after movement
                    this->environmentBoard[xCounter][yCounter]->Move();
                }
            }
        }

можно просто записать в виде

    //Iterate over environment
    for(int yCounter = 0; yCounter < 20; yCounter++)
        for (int xCounter = 0; xCounter < 20; xCounter++)
        {
            if (environmentBoard[xCounter][yCounter] != nullptr)
            {
                if (environmentBoard[xCounter][yCounter]->GetIdentifierTag() == 'o' && !(environmentBoard[xCounter][yCounter]->GetMoveAlternatorStatus()))
                {   
                    //Toggle moveAlternator
                    environmentBoard[xCounter][yCounter]->ToggleMoveAlternator();

                    //Move ant....Move function must return coordinate otherwise we attempt to increment/breed null pointer after movement
                    environmentBoard[xCounter][yCounter]->Move();
                }
            }
        }

См. этот вопрос stackoverflow.com для обсуждения того, когда следует использовать this->.

Инициализация

Конструктор организма можно было записать в организм.cpp как

organism::organism()
: moveAlternator{false}, location{0,0}, breedingCycle{0}
{
}

, чтобы инициализировать все переменные в классе организма.

ответил pacmaninbw 16 Maypm17 2017, 21:37:48
4

Конструкторы

Оба ant и doodlebug используйте ту же инициализацию, поэтому напишите логику в ваш базовый класс и просто перенаправьте конструктор. Поскольку у вас уже есть класс coordinates, используйте вместо него конструктор вместо развязанных параметров. Всегда старайтесь выразить идеи напрямую в коде .

organism::organism(coordinates const& coords, environment* env)
    : location(coords)
    , environmentPointer(env)
{}

ant(coordinates const& coords, environment* env)
    : organism(coords, env)
{
    identifierTag = 'o';
}
  

, но как бы инициализировать эти переменные-члены при создании экземпляра   экземпляр дочернего класса с частными членами в родительском   класс?

Это также отвечает на ваш вопрос. Пересылкой в ​​конструктор базового класса выполняется инициализация его элементов.

Доступ к среде

Многие из ваших методов в organism и его под-классы получают доступ к базовому массиву environment. Наверное, об этом говорил ваш профессор. Было бы лучше скрыть детали реализации environment, чтобы обеспечить методы для управления им. Организм не заботится об окружающей среде. Все, что он хочет сделать, это проверить, что находится на каком-то месте, и, возможно, переехать туда. Кроме того, во всех этих методах у вас есть ОГРОМНАЯ сумма - избыточный код. Следующим принципом является Не повторяйте . Определите логику, чтобы проверить, что находится на месте в классе environment. И логика получения смежного местоположения может быть помещена в класс coordinates, а также в environment. Таким образом, ваш код будет короче, яснее и менее связан.

void organism::RandomDirectionalMovement(int direction)
{
    auto newLocation = location.GetAdjacent(direction);
    if (environmentPointer->Move(this, newLocation))
        location = newLocation;
}

coordinates GetAdjacent(int direction) const
{
    switch (direction)
    {
        case 1:
            return { xCoordinate, yCoordinate - 1 };
        case 2:
            return { xCoordinate + 1, yCoordinate };
        case 3:
            return { xCoordinate, yCoordinate + 1 };
        case 4:
            return { xCoordinate - 1, yCoordinate };
    }
}

bool environment::IsValid(coordinates const& location) const
{
    return location.xCoordinate >= 0 && location.xCoordinate < 20
           && location.yCoordinate >= 0 && location.yCoordinate < 20;
}

bool environment::IsFree(coordinates const& location) const
{
    return IsValid(location) && environmentBoard[location.xCoordinate][location.yCoordinate] == nullptr;
}

bool environment::Move(organism* organism, coordinates const& location)
{
    if (organism && IsFree(location))
    {
        auto curLocation = organism->GetLocation();

        environmentBoard[curLocation.xCoordinate][curLocation.yCoordinate] = nullptr;
        environmentBoard[location.xCoordinate][location.yCoordinate] = organism;
        return true;
    }
    return false;
}

То же самое относится и к другим методам.

Не используйте магические числа

В вашем классе среды вы жестко закодировали его размер. Если вы хотите изменить свой размер для другого эксперимента, вам нужно изменить весь код и, возможно, забыть или перепутать некоторые, особенно если вы не хотите использовать квадратную панель. Поэтому сделайте их constexpr и обратитесь к их имени в вашем коде.

class environment
{
public:
    static constexpr int width = 20;
    static constexpr int height = 20;
private:
    organism* environmentBoard[width][height];
    // ...
}

Вы также жестко закодировали значения направления. Поместите их в перечисление, чтобы вы могли легко ссылаться на них по имени. Также определите значение тега в классах вашего организма, поэтому вы можете проверить вместо ant::tag вместо жестко заданного значения. Магические числа моих образцов выше также должны быть заменены, конечно.

class ant : public organism
{
public:
    static constexpr char tag = 'o';
}

Возможно, было бы еще лучше, если бы конструктор среды мог принимать два аргумента, обозначающие его размер. Таким образом, вы можете легко запускать разные эксперименты, не меняя код. Вам понадобится динамическая структура массива, например std::vector.

GenerateRandomStartingLocations

Этот метод будет иметь больший смысл, если он действительно вернет местоположение. Теперь он просто возвращает случайное число, это не то, что подразумевает название. Вы также используете его для создания случайного направления. Сделайте генерацию случайного числа свободной функцией в некотором пространстве имен приложений. Тогда эта функция может фактически вернуть местоположение. Поскольку ширина и высота среды известны, нам не нужны параметры. Кроме того, поскольку этот метод должен генерировать начальное местоположение, мы также можем проверить, занято ли сгенерированное местоположение или нет непосредственно в этом методе. Таким образом, вы избавитесь от большого количества дубликатов кодав вашем методе CreateStartPopulation.

coordinates environment::GenerateRandomStartingLocations()
{
    coordinates res;
    do
    {
        res.xCoordinate = Util::Random(0, width);
        res.yCoordinate = Util::Random(0, height);
    }
    while (environmentBoard[res.xCoordinate][res.yCoordinate] != nullptr);

    return res;
}

Упрощения

void organism::ToggleMoveAlternator()
{
    // make use of the not operator here, to get rid of the if 
    moveAlternator = !moveAlternator;
}

bool doodlebug::Death()
{
    // again get rid of the if and directyl return the expression
    return starveCounter == 3;
}

Использование циклов на основе диапазона может сделать ваши циклы более читабельными:

void environment::AntsAct()
{
    for (auto const& row : environmentBoard)
    {
        for (auto& orga : row)
        {
            if (orga && orga->GetIdentifierTag() == ant::tag)
            {
                orga->ToggleMoveAlternator();
                orga->Move();
            }
        }
    }
    // ...
}

Еще лучше, использование lambdas может еще больше повысить удобочитаемость.

void environment::LoopOrganisms(char tag, std::function<void(organism*)> func)
{
    for (auto const& row : environmentBoard)
    {
        for (auto& orga : row)
        {
            if (orga && orga->GetIdentifierTag() == tag)
                func(orga);
        }
    }
}

void environment::AntsAct()
{
    LoopOrganisms(ant::tag, [](auto && orga)
    {
        orga->ToggleMoveAlternator();
        orga->Move();
    });

    LoopOrganisms(ant::tag, [](auto && orga)
    {
        orga->IncrementBreedingCycle();
        if (orga->TimeToBreed())
            orga->Breed();
    });
}

Использовать = default

Когда ваши конструкторы /деструкторы не делают ничего особенного из созданных по умолчанию, явно пишут, что:

virtual ~organism() = default;

Используйте override

Если у вас есть базовый класс с методами virtual и их подклассами, используйте override.

void Move() override;   
bool TimeToBreed() override;

Это также предотвратит ошибки, когда вы хотите переопределить метод, но имеете опечатку в списке имен или параметров и фактически создаете новый метод. Вы получите ошибку, если нет виртуального базового метода.

Другие вещи Не используйте this для доступа к членам вашего класса. Это не обязательно в C ++ и просто загромождает код.

Считайте использование лучшего генерации случайных чисел, чем srand, например. те, которые представлены в <random>

ответил Sebastian Stern 16 Maypm17 2017, 22:25:46

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132