patterncppModerate
Linked list of bunny objects
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:
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
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
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
-
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
-
Distinguish local variables from member variables in some way. Often people use a leading
eg. In BunnyList.h
-
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
A few examples:
Const
You are not using const
eg. in BunnyList.h
Style
- You should name your files with the same name as the class (eg.
class RabbitNodewould 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 sameifblock 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 declareRabbitNodeand leave out the#include. After making the above change, in BunnyList.cpp, you need the definition ofRabbitNode, so the forward declaration you bring in from BunnyList.h won't be sufficient - you should now#include BunnyNode.hhere. 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.