Sign in to follow this  

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

This topic is 2543 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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[i][j]=0;
board[0][7]='*';
for(int i=0;i<=7;i++)
board[0][i]='*';
for(int i=0;i<=7;i++)
board[i][0]='*';
for(int i=0;i<=7;i++)
board[7][i]='*';
for(int i=0;i<=7;i++)
board[i][7]='*';

for(int i = 0;i<=5;i++)
board[2][i]='*';
for(int i=4;i<=7;i++)
board[i][2]='*';
for(int i=4;i<=7;i++)
board[i][4]='*';
for(int i=5;i<=7;i++)
board[4][i]='*';

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[i][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]

Share this post


Link to post
Share on other sites
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]

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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!

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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.)

Share this post


Link to post
Share on other sites
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[i][j]=0;
board[0][7]='*';
for(int i=0;i<=7;i++)
board[0][i]='*';
for(int i=0;i<=7;i++)
board[i][0]='*';
for(int i=0;i<=7;i++)
board[7][i]='*';
for(int i=0;i<=7;i++)
board[i][7]='*';

for(int i = 0;i<=5;i++)
board[2][i]='*';
for(int i=4;i<=7;i++)
board[i][2]='*';
for(int i=4;i<=7;i++)
board[i][4]='*';
for(int i=5;i<=7;i++)
board[4][i]='*';

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[i][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");
}

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites

//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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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!)

Share this post


Link to post
Share on other sites
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 ^_^

Share this post


Link to post
Share on other sites

This topic is 2543 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this