7
\$\begingroup\$

I am trying to find whether a given number is a perfect square or not by using just addition and subtraction.

Please review my code.

#include<stdio.h>
int check(int);
main()
{
    int N;


    printf("\n Enter the N:");
    scanf("%d",&N);
    if(check(N))
    {
        printf("\n[%d] Perfect Square:\n",N);

    }

    else
    {

        printf("\nNot perfect Square\n");

    }
}
int check(int n)
{
    int i=1;

    while(n>0)
    {
        n-=i;
        printf("[%d]",n);
        i+=2;
    }
    if(n==0)
        return 1;

    return 0;

}
\$\endgroup\$
  • 1
    \$\begingroup\$ Applying proper formatting to your code would be a good first step. \$\endgroup\$ – CodesInChaos Feb 10 '14 at 14:17
  • 1
    \$\begingroup\$ Since this is C++, you could use a proper bool and the check at the end simplifies to return n == 0; \$\endgroup\$ – CodesInChaos Feb 10 '14 at 14:21
  • \$\begingroup\$ There is nothing C++ in this code, so I am going to remove that tag. \$\endgroup\$ – syb0rg Feb 10 '14 at 14:43
14
\$\begingroup\$

Presentation

I have re-indented your code, I do not know how your code is on your side but you should definitly indent it properly. Also, you should remove useless lines.

Naming

check is not a good function name. isPerfectSquare would probably be easier for everyone to understand.

Documentation

Is it a good habit to add some documentation telling what your code is supposed to do. In your case, it could be nice to also tell the reader how it works.

The expression for the nth square number is n2. This is also equal to the sum of the first n odd numbers.

Types

You can use the bool type in C++.

\$\endgroup\$
7
\$\begingroup\$

Your code should only be 26 lines long. Most of the empty lines in your code are unnecessary.

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

static bool isPerfectSquare(int n) {

I changed the return type to bool, since the function either returns true or false.

  int i = 1;

  while (n > 0) {
    n -= i;
    i += 2;
  }
  return n == 0;
}

I also removed the printf statement, since that has been only for debugging. Instead of leaving these statements in the code, you should write unit tests.

int main(void) {

I changed the return type to int, since that is the required return type since the year 1989.

  int n;

  printf("Enter the number: ");

There should not be an extra newline and space in front of the word Enter. But there should be a space behind the colon.

  if (scanf("%d", &n) == 1) {

When the scanf fails, the program should not continue.

    if (isPerfectSquare(n)) {
      printf("The number %d is a perfect square.\n", n);
    } else {
      printf("The number %d is not a perfect square.\n", n);
    }

I changed the text messages, so that they don't contain unnecessary brackets. You wrote a colon at the end of the line, which suggests that some text follows. That was misleading, so I removed it.

  }
  return 0;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.