I made a grid, what do you think about it?

Hello!

I'm a beginner C++ programmer.
Yesterday I made the grid below, it was difficult to make. ( I'm not used YET to the intensive thinking when you make a program )

I want to improve my grid:

- Can you give me some advice about it? ( What can I do better, what are the mistakes I made, etc.. )
- I want to improve, can you tell me what I can add? Give me something to do! :)

Thank you!

ps: You can move your character ( S ) by typing w|a|s|d in the console ;)

CODE:

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
65
66
#include <iostream>
#include <cstdlib>
#include <time.h>

#define ONEUNIT 1
using namespace std;

/*POWERED BY: zapshe*/
void cycle();

int player = 0;

int Y = 4;  //height of the map
int X = 4;  //width of the map

int main()
{
    srand(time(NULL));
    int random = rand()%X*Y+1;

    while (true)
    {
    cycle();
    }
}

void cycle()
{
    char input;
    int i;
    for( i = ONEUNIT; i <= X*Y; i++ )
        {
            if ( player == i ) 
                printf("%3s", "S");
            else
                printf("%3s", "x");
            if ( i == X || i == X*2 || i == X*3 || i == X*4)
                cout << "\n";
        }
    cin >> input; //32767 is the MAX value of a stream.
    cin.ignore(1000, '\n'); //ignores 1000 characters after the character in input. 

    switch( input )
    {
        case 'a':
            player -= ONEUNIT;
            break;
        case 'd':
            player += ONEUNIT;
            break;
        case 'w':
            player -= X;
            break;
        case 's':
            player += X;
            break;
        default:
            cout << "Your input is invalid!";
            break;
    }

    if ( player < ONEUNIT )
        player = ONEUNIT;
    else if ( player >= i )
        player = X*Y;
}
Last edited on
Use code tags: code] /code] Put code between that and it'll be easier to read as you'll see.
Just start them off with [ (which I didn't so that it doesn't convert it). To bring it up, simply press the <> box (top left) on the left of the text box under "Format:" when you're replying.

First off, you're making the function call itself at the end! You're constantly creating new variables "input" and "i" every time you call the function and the old ones aren't destroyed.

What would be better is this (and this is what code looks like with code tags!):

1
2
3
4
5
6
7
8
9
10
int main()
{
     srand(time(NULL));
     int random = rand()%X*Y+1;

     while(true)
     {
          cycle();
     }
}


By using a loop instead, when the function "cycle" ends, the variables die and the memory is free to reuse. Unlike what you were doing, the function would NEVER end, leaving the variables using memory space while never going to be used. Then it would call itself over and over, piling up unused variables after each iteration!

Also, your integer "random" is never used.

The reason the program loops through multiple times if you input multiple characters is because cin takes in all input until it reaches white space. So if I input "saas", input will equal 's' and it'll go through. However, in the buffer, there's still "aas". So the next time it goes to cin >>, it'll see that there's already input there waiting to be used and it'll grab that instead of waiting for input from the user and input will equal 'a'. So it's not repeating for every input given, it's going through the program normally and then using the input left in the buffer instead of waiting for input from the user.

You can get rid of the extra input if you want, or you can leave it as is so that you can move the "S" several times in one go.

A good thing to do would be to add a "default" case for your switch that lets the user know they inputted a wrong value.

Anyway, good job! ^-^
Last edited on
Also, if you press "w" while it's all the way up it goes to the top left. If you press "s" while it's all the way down it goes bottom right. Is this what you wanted?

EDIT: I also forgot to say that you forgot the #include both #include <cstdlib> and #include <time.h>

Even if it runs fine for you without them, rand/srand and time() wont work on other compilers without them
Last edited on
Thank you very much!
I made the while loop and I made other changes :D

-I don't understand how do you clear the cin buffer.
I know about cin.ignore(); but if I understand correctly it just clears from "characters" like \t \n etc..
Last edited on
cin.ignore(100, '\n');
What this will do is ignore 100 characters or ignore up until it finds a '\n' (new line), which ever terminating condition comes first.

All cin.ignore() with no parameters will do is literally ignore 1 extra input. For example, now if I type "saas", it'll take the 's', ignore an 'a', finish everything then go back to cin >> where "as" still remains. Then take the next 'a' and ignore the final 's'.

So using those parameters will help you get rid of all extra input.

You can use either
1
2
3
4
std::cin.ignore(32767, '\n');
//or
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
//^ Last one needs #include <limits> 


The limit of characters you can have in the stream is 32767 so you can use that number or use the actual function that returns that number as shown in the second one. Either way, what these will do is ignore ALL input until a new line character is removed.
I understand now!
Thank you so much for your explanations, they really increased my C++ skill level.
I wish you the best of luck! :)
Last edited on
No problem, and I'll try to get suicide right the first time ^-^ Good luck with your learning!
Topic archived. No new replies allowed.