Public Group

# Is my code efficient?

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

## Recommended Posts

Hello, I made a simple program for my C class and just wanted to see what others thought about my code so I can improve. Is there a better way of doing what I was attempting? The program basically asks for an odd number which correlates to the size of the diamond which gets printed out

here's the code, any suggestions are appreciated!

#include <stdio.h>
#include <stdbool.h>

int main(int argc, const char * argv[])
{
int max_stars;
bool odd = false;

do {
printf("How many stars wide do you want your diamond size to be? (odd): ");
scanf("%d", &max_stars);

// checks for odd number
if(max_stars%2 == 1)
odd = true;
}while(odd == false);

int num_spaces = max_stars - 1;
int current_stars = 1;

// top half of diamond
for(int i = num_spaces; i > 0; i-=2) {
for(int i = 0; i < num_spaces/2; i++)
printf(" ");

for(int i = 0; i < current_stars; i++)
printf("*");

printf("\n");
current_stars += 2;
num_spaces -= 2;
}

// bottom half of diamond
for(int i = num_spaces; i <= max_stars; i+=2) {
for(int i = 0; i < num_spaces/2; i++)
printf(" ");

for(int i = 0; i < current_stars; i++)
printf("*");

printf("\n");
current_stars -= 2;
num_spaces += 2;
}

return 0;
}

Edited by NUCLEAR RABBIT

##### Share on other sites

You could eliminate the odd variable like this:

do {
// ...stuff
} while (max_stars % 2 == 0);


Then you also don't need to inlcude stdbool.h

I think the condition is pretty self explanatory. Alternatively you could factor this expression out into a function is_odd()

Also you probably want to check if max_stars is greater than zero.

##### Share on other sites

Your code will not compile in all compilers. You are using "int i" in every single for loop. The rest of the alphabet is jealous and wants to be included too.

I'm actually kind of surprised it compiles on any compiler. You're declaring "i" in inner loops while "i" is already in scope from the outer loop.

Other than that minor issue and the potential for negative inputs mentioned by madhed, everything looks good.

##### Share on other sites

You could eliminate the odd variable like this:

do {
// ...stuff
} while (max_stars % 2 == 1);


Then you also don't need to inlcude stdbool.h

I think the condition is pretty self explanatory. Alternatively you could factor this expression out into a function is_odd()

Also you probably want to check if max_stars is greater than zero.

sweet thanks! made those tweeks!

Your code will not compile in all compilers. You are using "int i" in every single for loop. The rest of the alphabet is jealous and wants to be included too.

I'm actually kind of surprised it compiles on any compiler. You're declaring "i" in inner loops while "i" is already in scope from the outer loop.

Other than that minor issue and the potential for negative inputs mentioned by madhed, everything looks good.

oops, didn't realize I was doing that haha thanks for pointing that out I shall make it more diverse!

##### Share on other sites

Your code will not compile in all compilers. You are using "int i" in every single for loop. The rest of the alphabet is jealous and wants to be included too.

I'm actually kind of surprised it compiles on any compiler. You're declaring "i" in inner loops while "i" is already in scope from the outer loop.

Probably more detail than I should go into here, but: the code as written is no no danger of failing to compile. Although reusing the same variable name so many times is confusing (and leads to the potential for logic errors in this particular program), it is perfectly legal.

The declaration of 'int i' inside a for statement doesn't last beyond the end of the for loop, so redeclaring it in the next loops is both legal and necessary (note that older compilers will let the variable live on for the rest of the function, so not redeclaring it will generally work too).

Declaring 'int i' again in the inner for loops is also legal, as the inner declaration will hide the outer declaration. This means that changes to i inside the inner loop will affect the inner i, and outside of the inner loop will affect the outer i.

• ### Game Developer Survey

We are looking for qualified game developers to participate in a 10-minute online survey. Qualified participants will be offered a \$15 incentive for your time and insights. Click here to start!

• 13
• 18
• 15
• 10
• 9