need help with best programming practice

[xjoexripx]

I am starting to care more about how I structure my programs, and want to be as efficient as possible and need some tips. I'm not too far, about half-way through classes in Samsteachyourself C++ seventh edition.

I will include all my .h files first and then my main program and then all my .cpp files that are based on the .h files and they will be in pastebin

This program is based off of beginner challenge dungeon crawl which I upgraded as more of a challenge for myself.

 //Board.h
http://pastebin.com/Nn6jpxm2

//Obstacle.h
http://pastebin.com/Cr1VBxt1

//Player.h
http://pastebin.com/nmmwHsRu

//Main.cpp   this is the main part of my program
http://pastebin.com/6baDa8Sz

//Board.cpp
http://pastebin.com/Bfcner7L

//Obstacle.cpp
http://pastebin.com/GsL4HAv7

//Player.cpp
http://pastebin.com/XxHKUHm5

[TheIdeasMan]

Hi,

Welcome to Cplusplus :+)

http://www.cplusplus.com/articles/z13hAqkS/

Just mentioned this, one can make a URL link by quoting the web address.

Ok, with your code.

I like to use *.hpp as opposed to *.h for header files, it means there is cpp code in the file.

Looking at Board.h:

Only include header files you need.

Don't have using namespace std;, put std:: before each std thing, believe me it's the best way. Google to what is bad about using namespace std;

max_col and max_row should be unsigned, or std::size_t , or one of the unsigned types found in <cstdint> header.

Line 24: Put 1 statement per line

Looking at Board.cpp:

Is Obstacle needed in this file?

Board::initialize is a little misleading, it only initialises 1 square

Board::display is a little misleading, it only displays 1 square

Board::get_col and Board::get_row return their respective max size, so the function name is misleading.

You could have a function called ValidSquare say, so you don't have this repeated:

get_square(col, row) == blank && col >= 0 && col < max_col && row >= 0 && row < max_row

Looking at Obstacle.h:

Remove extraneous includes.

You have the board sizes again, an object should be responsible for it's own info, and not be concerned with other objects info. The same goes for functions.

Looking at Obstacle.cpp:

One of the ideas with ctors is to set all the class members and invariants, this is a bit confused at the moment, because you all this other stuff from other classes.


Hopefully this is enough to get you started :+)

[MiiNiPaa]

Use member initialization list instead assigning members in the body of class.
http://en.cppreference.com/w/cpp/language/initializer_list
http://stackoverflow.com/questions/926752/why-should-i-prefer-to-use-member-initialization-list
http://www.learncpp.com/cpp-tutorial/101-constructor-initialization-lists/

Be const-correct: mark member function which do not change state of class as const.

Be const-correct: pass objects you do not modify by const reference.

Avoid naked new and delete: use STL containers or at least smart pointers.

[xjoexripx]

Hey TheIdeasMan,
Thank you for your input I didnt even realize that I still had those headers included where they shouldn't be. Believe it or not before I cleaned it up a bit, I had these classes wayyyyy jumbled. I would have functions whose parameters would be around 8 different things. It was atrocious haha, and that was really great advice for .hpp! I never thought of that. Could you expand upon what you said about including board sizes under Obstacle.h, you could have a better way than what I am doing. My logic for class player and class obstacle is to keep track of where the user is, the traps are, and where the ai are. Before I had to have a function within board that would have to search the entire board for each piece. Since the program can easily find where the user, ai, and traps are the board takes that info and places them in the display as needed. If you have a better way please let me know, the tips you gave me were really good. And I am also not sure what ctors are or invariants, I am half-way through the chapter for classes so I may come upon them soon.

MiiNiPaa,
thank you for the links I'll be sure to read them! By changing state of class are you talking about function parameters that take the class by reference>

[MiiNiPaa]

By changing state of class are you talking about function parameters that take the class by reference>

I am talking about member functions which do not change class members:
int player::getcol() { return Col; } It does not changes anything in class, so it should be const member.

[TheIdeasMan]

Could you expand upon what you said about including board sizes under Obstacle.h,

If an object needs to find out something about another object, then make use of that object's interface (it's public functions) to return a value.

And I am also not sure what ctors are or invariants

ctor is an abbreviation for constructor, also dtor is destructor.

Class invariants are conditions relating to the value of member variables that must satisfied at all times, in order for the object to remain in a valid state.

To analyse that concept, first think about what valid ranges there are for each of the class members. It may be simply that the value must be positive, or there is an actual range of values that are valid. Then consider that several variables may be interdependent - as in a circular arc, the 3 points on the arc cannot be in a straight line, the distance from any point to the centre must be consistent with the radius.

So these invariants must be checked upon construction of the object, and any time something changes. This brings us to pre conditions - that is valid values for arguments to functions, including constructors. Also post conditions which are usually a valid return value.

Good Luck!!