patterncMinor
Simple treasure hunter
Viewed 0 times
simplehuntertreasure
Problem
I have made a program that reads 3 numbers:
It then reads a string (which symbolizes the treasure chests), being each character either
When the treasure hunter moves, he loots the treasures which he stands in, and in the end the program tells how many he got.
The requirements are the following:
1 ≤ number of treasure chests ≤ 100 000
1 ≤ treasure hunter's position ≤ number of treasure chests
1 ≤ number of moving instructions ≤ 100 000
the number of steps is always higher than 0
the treasure hunter never goes beyond the treasure chests
My working code which I wish to improve in every way possible is the following:
Example of input and output:
I:
O:
- number of treasure chests
- treasure hunter's position
- number of moving instructions
It then reads a string (which symbolizes the treasure chests), being each character either
T (chest has a treasure) or V (chest is empty). After that, it reads the moving instructions in the format %c %d, being the first character (direction) either E(left) or D(right) and then the number of steps.When the treasure hunter moves, he loots the treasures which he stands in, and in the end the program tells how many he got.
The requirements are the following:
1 ≤ number of treasure chests ≤ 100 000
1 ≤ treasure hunter's position ≤ number of treasure chests
1 ≤ number of moving instructions ≤ 100 000
the number of steps is always higher than 0
the treasure hunter never goes beyond the treasure chests
My working code which I wish to improve in every way possible is the following:
#include
#include
int main() {
char e;
int a, b, c, f, g = 0, i, j;
scanf("%d %d %d", &a, &b, &c);
char *d = malloc(a * sizeof(char));
scanf("%s", d);
for(i = 0; i < c; i++) {
scanf(" %c %d", &e, &f);
for(j = 0; j <= f; j++) {
if(d[b - 1] != 'V') {
d[b - 1] = 'X';
}
if(e == 'E') {
b++;
} else {
b--;
}
}
}
for(i = 0; i < a; i++) {
if(d[i] == 'X') {
g++;
}
}
printf("%d\n", g);
return 0;
}Example of input and output:
I:
5 3 3
TVTTT
D 1
E 2
D 3O:
3Solution
Variable names
Variables such as
Bug
You don't allocate enough space for your string here:
Suppose
Also, it is unsafe to just call
Note also that
No need for second loop
You have a second loop to count the number of
Sample rewrite
Here is a rewrite of your program which fixes all of the above:
Variables such as
a, b, c, d are meaningless. Use names with meaning such as position, numChests, chestValues, etc. Sure, this is just a small program with 8 total variables, but you should get in the habit of producing readable code and not just correct code.Bug
You don't allocate enough space for your string here:
char *d = malloc(a * sizeof(char));
scanf("%s", d);Suppose
a had value 5, as in the example. The string "TVTTT" has 5 characters plus a terminating null character, for a total of 6 characters. So you need to allocate an extra byte for the null character when you call malloc().Also, it is unsafe to just call
scanf() because if the input string happened to be wrong and contain more than 5 characters, you would overflow your buffer. To be safe, you can use fgets(), which limits the number of characters it writes to your buffer:char *chestValues = malloc(numChests+1);
fgets(chestValues, numChests+1, stdin);Note also that
sizeof(char) is always equal to 1, so you can omit that.No need for second loop
You have a second loop to count the number of
X in your chest array. But you could have counted the number of chests found that were T in your first loop and then you wouldn't need the second loop at all.Sample rewrite
Here is a rewrite of your program which fixes all of the above:
#include
#include
int main(void)
{
int numChests = 0;
int position = 0;
int numMoves = 0;
int numFound = 0;
char *chestValues = NULL;
scanf("%d %d %d\n", &numChests, &position, &numMoves);
chestValues = malloc(numChests+1);
fgets(chestValues, numChests+1, stdin);
for (int i = 0; i < numMoves; i++) {
char direction = 0;
int distance = 0;
scanf(" %c %d", &direction, &distance);
for (int j = 0; j <= distance; j++) {
if (chestValues[position - 1] == 'T') {
chestValues[position - 1] = 'X';
numFound++;
}
position += (direction == 'E' ? 1 : -1);
}
}
printf("%d\n", numFound);
return 0;
}Code Snippets
char *d = malloc(a * sizeof(char));
scanf("%s", d);char *chestValues = malloc(numChests+1);
fgets(chestValues, numChests+1, stdin);#include <stdio.h>
#include <stdlib.h>
int main(void)
{
int numChests = 0;
int position = 0;
int numMoves = 0;
int numFound = 0;
char *chestValues = NULL;
scanf("%d %d %d\n", &numChests, &position, &numMoves);
chestValues = malloc(numChests+1);
fgets(chestValues, numChests+1, stdin);
for (int i = 0; i < numMoves; i++) {
char direction = 0;
int distance = 0;
scanf(" %c %d", &direction, &distance);
for (int j = 0; j <= distance; j++) {
if (chestValues[position - 1] == 'T') {
chestValues[position - 1] = 'X';
numFound++;
}
position += (direction == 'E' ? 1 : -1);
}
}
printf("%d\n", numFound);
return 0;
}Context
StackExchange Code Review Q#159372, answer score: 3
Revisions (0)
No revisions yet.