HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Linked list of bunny objects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listlinkedobjectsbunny

Problem

The exercise is the bunny linked list exercise; the last beginner exercise from here.

I'm looking for feedback on absolutely everything that could make me a better programmer:

  • Syntax



  • Optimization



  • Form



  • Functions or variables naming



  • Bugs



  • Performance



  • Code structure



Keep in mind that I haven't completed all aspects of the exercise related to it running in real-time or outputting events to a file.

main.cpp

#include 
#include 
#include 
#include 

#include "BunnyNode.h"
#include "BunnyList.h"

//BUGS
//Generate at least one female and one male on start

//TO DO
//Replace names with vectors(?) generated from separate .txt file
//Output all turn events, birth, turn mutant, dies
//Output all turns to file

using std::cin;
using std::cout;
using std::endl;

int main()
{
    srand(time(0));

    // Create rabbit LinkedList
    RabbitList * colony = new RabbitList;

    bool terminate = false;
    int turns = 0;

    //Add 5 rabbits to list for the first round
    colony->startCheck(turns);

    while(!terminate)
    {
        colony->printList();
        cout incrementRabbitAge();

        //Add babies
        colony->addBabies();

        //Each mutant will convert one other bunny each turn
        colony->mutantConversion();

        //Check if colony is 0 or 1000
        colony->sizeCheck(terminate);

        cout mutantCount() printSize();

        ++turns;

        if(colony->getColonySize() != 0)
        {
            cout > choice;

            if(choice == 'k')
                colony->purge();
        }
    }

    cout << "\nTOTAL TURNS: " << turns << endl;

    delete colony;

    return 0;
}


BunnyList.cpp

```
#include
#include
#include

#include "BunnyList.h"
#include "globals.h"

using std::string;
using std::cout;
using std::endl;

RabbitList::RabbitList(): head(NULL),size(0)
{}

RabbitList::~RabbitList()
{}

int RabbitList::randomGeneration(int x)
{
return rand() % x;
}

void RabbitList::generateRandomFeatures(RabbitNode

Solution

Not a full review, but just some general comments on style and other C++ idioms. Note that I only looked at some of the code and wasn't looking at correctness.

Style

  • You should name your files with the same name as the class (eg. class RabbitNode would be in RabbitNode.h and RabbitNode.cpp)



  • Always use braces {} on if/else and while blocks. Yes, you don't have to use them if it's a one-liner, but it avoids bugs in the future when you add more lines to the same if block and forget to add the braces



-
Put items in the constructor's initialization list on separate lines. This makes for easier reading when there are many member variables, and it makes diffs cleaner when you make changes

RabbitNode::RabbitNode()
    : next(NULL)
    , firstName("none")
    , lastName("none")
    , colour("none")
    , age(0)
    , radioactive_mutant_vampire_bunny(0)
{}


-
Distinguish local variables from member variables in some way. Often people use a leading _ or m_. This makes it easier to read code and know if you're touching locals or members, and it also makes using auto-complete features of your IDE easier. If you're coding and want to see all the members of your class to auto-complete, just start with m_ for example to get the list.

eg. In BunnyList.h

private:
    RabbitNode* m_head;
    int m_size;


-
Avoid globals. There are cases where they should be used, but generally should be avoided. Especially if you're just starting out, you should actively develop the habit to avoid them. In some cases, some of the constants you've defined in your Globals.h file are only used in one file, so just move the constants there. In most other cases, you can avoid globals by passing them in as parameters to functions or class constructors.

Includes / Forward declaration

The general rule is, only use an #include for what you need in that single file, and use forward declaration wherever possible instead of #include.

A few examples:

  • In BunnyNode.h, you aren't using anything from ` here, so don't include it



  • In BunnyList.h, you don't actually need the definition of RabbitNode since you only have a pointer member and pointer parameters. This means you can forward declare RabbitNode and leave out the #include. After making the above change, in BunnyList.cpp, you need the definition of RabbitNode, so the forward declaration you bring in from BunnyList.h won't be sufficient - you should now #include BunnyNode.h here. Check out this StackOverflow question for info on how/when to use forward declarations.



  • I believe you're not using anything from in BunnyList.cpp, so remove that #include



Const

You are not using
const keyword anywhere. Using const can not only prevent bugs, but also it makes the programmers intentions clear about how variables and functions are going to be used.

In general, if a class member function will not change the state its member variables, it should be declared
const. Note that this needs to be put in both the header and implementation file.

eg. in BunnyList.h

void printSize() const;


eg. in BunnyList.cpp

void RabbitList::printSize() const
{
    if(head)
        cout << "The colony's size is currently : " << size << endl;
}


In general, variables (including member variables) should be declared
const` if they will never change value. This goes for parameters too.

eg. in BunnyList.h

// rabbit will not be altered in this function, so make it const
// while we're at it, make the function const since this function will not alter any member variables
bool ageCheck(const RabbitNode * rabbit) const;

Code Snippets

RabbitNode::RabbitNode()
    : next(NULL)
    , firstName("none")
    , lastName("none")
    , colour("none")
    , age(0)
    , radioactive_mutant_vampire_bunny(0)
{}
private:
    RabbitNode* m_head;
    int m_size;
void printSize() const;
void RabbitList::printSize() const
{
    if(head)
        cout << "The colony's size is currently : " << size << endl;
}
// rabbit will not be altered in this function, so make it const
// while we're at it, make the function const since this function will not alter any member variables
bool ageCheck(const RabbitNode * rabbit) const;

Context

StackExchange Code Review Q#20407, answer score: 10

Revisions (0)

No revisions yet.