C++

Forum

 
How does my code look thus far? I have very few outlets for good advice and criticisms. Be gentle.

closed account 3R5fjE8b (226)
I am creating a game, all I have so far is the navigation of areas.
I will have multiple arrays linked at certain points to move around maps.

Can anyone give me ideas/criticisms? Be gentle I am entirely self-taught.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
 
#include <iostream>
using namespace std;

string home_town[5][5] = { // BEGIN MAPS < ----
{ "wall", "wall", "wall", "wall", "wall" },
{ "wall", "farm", "dock", "wall", "wall" },
{ "wall", "yard", "wall", "path", "exit" },
{ "wall", "home", "path", "path", "wall" },
{ "wall", "wall", "wall", "wall", "wall" } };

class CHARACTER {//BEGIN CHARACTER CLASS < ----
      public:
             CHARACTER::CHARACTER();
             void setLocation(string map[][5]);
             void displayLocation(string map[][5]);
             void canMove(string map[][5]);
             bool move(string map[][5], string decision);
      private:
              int xCoord, yCoord;
              string currentLocation;
              string username;
              }; 
              
CHARACTER::CHARACTER(){
xCoord = 3;//start location
yCoord = 1;}

void CHARACTER::setLocation(string map[][5]){
currentLocation = map[xCoord][yCoord];}

void CHARACTER::displayLocation(string map[][5]){
cout << currentLocation << endl;}
              
void CHARACTER::canMove(string map[][5]){
cout << "You can move in the following directions:" << endl;
if (map[xCoord - 1][yCoord] != "wall")
   {cout << "North" << endl;}
if (map[xCoord + 1][yCoord] != "wall")
   {cout << "South" << endl;}
if (map[xCoord][yCoord + 1] != "wall")
   {cout << "East" << endl;}
if (map[xCoord][yCoord - 1] != "wall")
   {cout << "West" << endl;} }

bool CHARACTER::move(string map[][5], string decision){
if ( decision == "north" && map[xCoord - 1][yCoord] != "wall"  || decision == "North" && map[xCoord - 1][yCoord] != "wall" )
   { xCoord = xCoord - 1; setLocation(map); return true;}
else if ( decision == "south" && map[xCoord +1][yCoord] != "wall" || decision == "South" && map[xCoord +1][yCoord] != "wall" )
     { xCoord = xCoord + 1; setLocation(map); return true; }
else if ( decision == "east" && map[xCoord][yCoord + 1] != "wall" || decision == "East" && map[xCoord][yCoord + 1] != "wall" )
     { yCoord = yCoord + 1; setLocation(map); return true; }
else if ( decision == "west" && map[xCoord][yCoord - 1] != "wall" || decision == "West" && map[xCoord][yCoord - 1] != "wall" )
     { yCoord = yCoord - 1; setLocation(map); return true; }
else { cout << "You can't move in that direction." << endl; return false; }
}


int main() // BEGIN MAIN FUNCTION HERE < ----
{
CHARACTER hero;

system("PAUSE");
return 0;
}
xismn (961)
I have some comments on style. Feel free to ignore them, but hopefully you'll find these useful.

1.) You might want to stick with a more consistent brace-placing style. In some places your beginning and closing braces are on the same line, sometimes the closing brace is at the end of a line of actual code (line 26 for example), and sometimes it's mixed (line) 55.

Having a consistent style in terms of placing braces makes your code easier to read - it becomes more clear where a function or a control structure ends.

Some people prefer:
1
2
3
4
 
void function()
{
	//code
}



Others prefer:
1
2
3
 
void function() {
	//code
}


2.) Make your comments useful. Unless you're really struggling to remember where your program begins, and where your classes are declared, comments such as "X BEGINS HERE" don't really make any difference. Sometimes it's hard to know when exactly to comment a line of code. I can't think of any particular example right now, but I'll try to come up with some later, and I'll update this post.

3.) If you're working with other people, they'll hate you if you all-caps the names of all your classes. I'm not saying you're a bad person - there was a time when I too, all-capped all the names of my classes - that's the code I'm embarrassed to look at now :(
There a several accepted styles - here's a link that I think is good to look at in your case:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Type_Names

It's the so called "Google style guide", which is basically a list of commonly accepted industry standard styles for all sorts of things.
closed account 3R5fjE8b (226)
Thank you very much for taking the time to comment. I should definitely get in the habit of using a consistent bracing style, and thank you for the advice on all caps classes, I had no idea about that. I was going to create an external class file but decided to just keep it all in one page while I am working on it. Other then that is the code itself fairly sound?
closed account 3R5fjE8b (226)
Took your advice to heart, thank you!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
 
#include <iostream>
using namespace std;

string home_town[5][5] = {
{ "wall", "wall", "wall", "wall", "wall" },
{ "wall", "farm", "dock", "wall", "wall" },
{ "wall", "yard", "wall", "path", "exit" },
{ "wall", "home", "path", "path", "wall" },
{ "wall", "wall", "wall", "wall", "wall" } };

class character {
      public:
             character::character();
             void setLocation(string map[][5]);
             void displayLocation(string map[][5]);
             void canMove(string map[][5]);
             bool move(string map[][5], string decision);
      private:
              int xCoord, yCoord;
              string currentLocation;
              string username;}; 
              
character::character()
{xCoord = 3;
yCoord = 1;}

void character::setLocation(string map[][5])
{currentLocation = map[xCoord][yCoord];}

void character::displayLocation(string map[][5])
{cout << currentLocation << endl;}
              
void character::canMove(string map[][5])
{cout << "You can move in the following directions:" << endl;
if (map[xCoord - 1][yCoord] != "wall")
   {cout << "North" << endl;}
if (map[xCoord + 1][yCoord] != "wall")
   {cout << "South" << endl;}
if (map[xCoord][yCoord + 1] != "wall")
   {cout << "East" << endl;}
if (map[xCoord][yCoord - 1] != "wall")
   {cout << "West" << endl;} }

bool character::move(string map[][5], string decision)
{if ( decision == "north" && map[xCoord - 1][yCoord] != "wall"  || decision == "North" && map[xCoord - 1][yCoord] != "wall" )
   { xCoord = xCoord - 1; setLocation(map); return true;}
else if ( decision == "south" && map[xCoord +1][yCoord] != "wall" || decision == "South" && map[xCoord +1][yCoord] != "wall" )
     { xCoord = xCoord + 1; setLocation(map); return true; }
else if ( decision == "east" && map[xCoord][yCoord + 1] != "wall" || decision == "East" && map[xCoord][yCoord + 1] != "wall" )
     { yCoord = yCoord + 1; setLocation(map); return true; }
else if ( decision == "west" && map[xCoord][yCoord - 1] != "wall" || decision == "West" && map[xCoord][yCoord - 1] != "wall" )
     { yCoord = yCoord - 1; setLocation(map); return true; }
else { cout << "You can't move in that direction." << endl; return false; }}


int main()
{character hero;

system("PAUSE");
return 0;
}
closed account 3R5fjE8b (226)
Am I going about what I'm trying to accomplish in a decent manner?
Topic archived. No new replies allowed.