Skip to main content
GameDev.net gamedev.net
🔒 Locked

i'm so proud right now, Tell me how this c++ is(it's a mess)

Started by link161 Dec 27, 2010 at 8:27 PM 14 replies 3.3k views
Original Post
link161
link161
ok I made this program, my first time with classes tell me how I can shape it up

#include<iostream>#include<string>#include<conio.h>using namespace std;class mboard{private:	char board[8][8];public:	mboard(){		cout<<"\nCreating the board...\n\n";	}	~mboard(){	}	//setting the positions	void setb(char pl,int plx,int ply){		for(int i=0;i<=7;i++)			for(int j=0;j<=7;j++)				board[j]=0;		board[0][7]='*';		for(int i=0;i<=7;i++)		board[0]='*';	for(int i=0;i<=7;i++)		board[0]='*';	for(int i=0;i<=7;i++)		board[7]='*';	for(int i=0;i<=7;i++)		board[7]='*';	for(int i = 0;i<=5;i++)		board[2]='*';	for(int i=4;i<=7;i++)		board[2]='*';	for(int i=4;i<=7;i++)		board[4]='*';	for(int i=5;i<=7;i++)		board[4]='*';	board[ply][plx]=pl; //setting the player position			}	//Drawing the board	void drawboard(){		for(int i=0;i<=7;i++){			for(int j=0;j<=7;j++)				cout<<board[j];		cout<<endl;		}	}	//checking for collision based on the movement key from the movement function	bool coll(int x,int y,char move){									if( move=='w')				if(board[y-1][x]!='*' )				return false;							if( move=='s')				if(board[y+1][x]!='*' )				return false;			if( move=='a')				if(board[y][x-1]!='*' )				return false;						if( move=='d')				if(board[y][x+1]!='*' )				return false;					        return true;	}};int main(){	int plx=3,ply=6;	char pl='0',sel;	mboard mainb;		//main loop while the selection is not 'q'	do{	mainb.setb(pl,plx,ply);	mainb.drawboard();	cout<<endl<<"x= "<<plx<<" y= "<<ply<<endl<<endl;	sel=getch();	//movement code	if(sel=='w'&&mainb.coll(plx,ply,sel)==false)		ply--;	if(sel=='s'&&mainb.coll(plx,ply,sel)==false)		ply++;		if(sel=='a'&&mainb.coll(plx,ply,sel)==false)		plx--;		if(sel=='d'&&mainb.coll(plx,ply,sel)==false)		plx++;	system("cls");	}while(sel!='q');	system("pause");}


[Edited by - link161 on December 27, 2010 9:10:56 PM]
wicked357
wicked357
It would get better results if you put your code in source tags. Right now it isn't the easiest to read and kinda hurts the eyes.
JasonR
JasonR
First you can put your code into source blocks so it isn't a big non-indented mess. :)

Good job for first time. Off to a good start. keep doing and learning. Get a good book on object oriented programming and you'll be right on track.

(EDIT: Ninja'd like a champ! )
[/source]
link161
link161
ha sorry about that, here that should be better. So I guess next I want to be able to update the screen without the input. and hopefully soon, a pong game than pacman clone.
wicked357
wicked357
Just my personal opinion and how I prefer to code is don't be afraid to use brackets, it can help to read your code for others and yourself especially when you have a lot of nested statements and loops.

so for example instead of
for(int x = 0; x < 10; x++)    for(int y = 0; y < 10; y++)        that[x][y] = 0


to be honest one isn't so bad, but when you have lots it can start to look messy. So instead of the above, you could try.
for(int x = 0; x < 10; x++){    for(int y = 0; y < 10; y++)    {        that[x][y] = 0    }}

Again, this is just my personal opinion on making the readability of the code easier for the reader.

EDIT** Also congrats on using classes for the first time!
link161
link161
Actually wicked, I completely agree with you, it wasn't until I took this programming class last semester that I got in to that habit. Which just came from practicing ifs so much and doing the exercises in class to finish em' first, it's my speedy programming habit lol
Zakwayda
Zakwayda
I don't know if you're still looking for feedback, but if so, perhaps you could repost your code. (For some reason, adding 'source' tags to existing text - or something like that - causes angle brackets to be displayed as their HTML equivalents, which makes the code difficult to read.)
Alpha_ProgDes
Alpha_ProgDes
I'll do it for him:
#include<iostream>#include<string>#include<conio.h>using namespace std;class mboard{private:	char board[8][8];public:	mboard(){		cout<<"\nCreating the board...\n\n";	}	~mboard(){	}	//setting the positions	void setb(char pl,int plx,int ply){		for(int i=0;i<=7;i++)			for(int j=0;j<=7;j++)				board[j]=0;		board[0][7]='*';		for(int i=0;i<=7;i++)		board[0]='*';	for(int i=0;i<=7;i++)		board[0]='*';	for(int i=0;i<=7;i++)		board[7]='*';	for(int i=0;i<=7;i++)		board[7]='*';	for(int i = 0;i<=5;i++)		board[2]='*';	for(int i=4;i<=7;i++)		board[2]='*';	for(int i=4;i<=7;i++)		board[4]='*';	for(int i=5;i<=7;i++)		board[4]='*';	board[ply][plx]=pl; //setting the player position			}	//Drawing the board	void drawboard(){		for(int i=0;i<=7;i++){			for(int j=0;j<=7;j++)				cout<<board[j];		cout<<endl;		}	}	//checking for collision based on the movement key from the movement function	bool coll(int x,int y,char move){									if( move=='w')				if(board[y-1][x]!='*' )				return false;							if( move=='s')				if(board[y+1][x]!='*' )				return false;			if( move=='a')				if(board[y][x-1]!='*' )				return false;						if( move=='d')				if(board[y][x+1]!='*' )				return false;					        return true;	}};int main(){	int plx=3,ply=6;	char pl='0',sel;	mboard mainb;		//main loop while the selection is not 'q'	do{	mainb.setb(pl,plx,ply);	mainb.drawboard();	cout<<endl<<"x= "<<plx<<" y= "<<ply<<endl<<endl;	sel=getch();	//movement code	if(sel=='w'&&mainb.coll(plx,ply,sel)==false)		ply--;	if(sel=='s'&&mainb.coll(plx,ply,sel)==false)		ply++;		if(sel=='a'&&mainb.coll(plx,ply,sel)==false)		plx--;		if(sel=='d'&&mainb.coll(plx,ply,sel)==false)		plx++;	system("cls");	}while(sel!='q');	system("pause");}
Beginner in Game Development?  Read here. And read here.  
link161
link161
Oh, thank you alpha. Yes I'm always looking for more feedback, or things you would have done differently and why etc. Just to get a better feel for what direction to take when I rewrite all this
Fuji
Fuji
Well, it's a bit hard to read. Documentation and spacing are important (if not others, for yourself and your professors). Most of it is just formatting, but it's still fairly clear what the program is and how it works. I know how it's like to work with classes in c++ (it's not the nicest thing in the world), and this is certainly a start. You seem to have followed proper OO design, too (unless I missed something). Keep working at it.

On a side note, just to get a scope of college/university programming, what did you cover in your previous programming class?
C++: Where your friends have access to your private members
Alpha_ProgDes
Alpha_ProgDes 
//Maybe someone should correct me but,//shouldn't this:cout << "Text\n";//actually be this:cout << "Text" << endl;


Also (again, anyone please correct me) shouldn't the constructor just call setb() since it's supposed to be creating the board? Or if the board can change, have the constructor call a function to set the size of the board and number of players.
Beginner in Game Development?  Read here. And read here.  
Concentrate
Concentrate
Quote:
Original post by Alpha_ProgDes
*** Source Snippet Removed ***

Also (again, anyone please correct me) shouldn't the constructor just call setb() since it's supposed to be creating the board? Or if the board can change, have the constructor call a function to set the size of the board and number of players.


The second one does the first one + flushes the stream. So if he didn't need to flush the stream then using '\n' is ok.
link161
link161
Thanks, fuji, I'll work on my readability. I just started working towards my computer science degree and my intro class covered basic expressions, data types, loops, and functions. I'm doing all this for prep on intermediate programming coming this spring that will cover classes, intro to data structures etc.

Alpha, as far as I know(which is still pretty basic) "\n" and endl both just create new lines and I haven't had a problem with using them back and forth yet.


As for using the constructor, I'm still getting used to using the class system and wasn't even thinking of putting the actual creation of the board in there, which actually seems a lot more efficient than recreating it every time I redraw it. hmm, I'm working on rewriting now, when I'm done I'll post it. Any more suggestions?

What about collision, how can I put that in a function where it can check the private member of the class without redefining it in the function with the constructor?
Steve132
Steve132
One thing I might suggest: nice job on the loops and all, and in general looping repetitive code is a good thing, but why didn't you do something like:
char board[8][9]={"********","*      *","****** *","*      *","* * ****","* * *  *","* * *  *","********"};


Its a crapton simpler to read and to modify then lots of loops. Part of learning programming is learning when to make something complex and extensible and when to KISS (keep it simple!)
link161
link161
Lol, That's a good question, 2d arrays(along with classes) are pretty much brand new to me. I knew I could bracket to default it all to one character but not every one like that, I, for some reason, was under the assumption I had to do it one by one, hence the loop. But.. Now that I know the method for that, I will indeed update lol Thank you ^_^

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.