patterncppMajor
Beehive numbers - using goto in C++
Viewed 0 times
numbersgotousingbeehive
Problem
I understand that using
This is my code for SPOJ. I know this does not reduce too many lines of code, but in a big project, it potentially could.
What should I do in such situations? Is there a way to do this by making a function call?
goto in C++ code is strictly unadvised, but sometimes, it really reduces the number of lines of code like in the following case.This is my code for SPOJ. I know this does not reduce too many lines of code, but in a big project, it potentially could.
#include
#include
#include
#include
using namespace std;
bool debug=false;
typedef long long int lld;
typedef unsigned long long int llu;
int main(int argc , char **argv)
{
if(argc>1 && strcmp(argv[1],"DEBUG")==0) debug=true;
lld n,val;
long double sq;
while(true){
scanf("%lld",&n);
if(n==-1)break;
n -= 1;
if(n%3!=0)goto exit;
n/=3;
sq=4*n+1;sq=sqrt(sq);
if(sq-(int)sq!=0)goto exit;
n=sq;
if(n%2!=1)goto exit;
printf("Y\n");
continue;
exit:
printf("N\n");
continue;
}
return 0;
}What should I do in such situations? Is there a way to do this by making a function call?
Solution
Your use of
Before addressing the core concern about
-
Your compiler should have warned you:
You do compile with warnings enabled, right?
-
-
-
The
-
As you suspected, a function call would help. Your
That provides two very important improvements over your code:
-
You can see at a glance what the purpose and structure of your program are. The clutter within the loop is all gone, replaced by a function whose purpose is obvious. The loop is properly structured — the phony
-
There is now proper separation of concerns.
When the calculation code is in its own function rather than embedded in the loop, the flow of control can be expressed so much better!
I don't understand the mathematics behind your code; I just transformed the code mechanically.
goto is wholly unjustified, not just because goto is taboo, but because your code has flow-of-control that is hard to follow. Furthermore, the use of goto is not even an effective way to achieve your goal of compactness.Before addressing the core concern about
goto, I'd like to point out that there is a lot of junk in your code:-
Your compiler should have warned you:
beehive.cpp:12:11: warning: unused variable 'val' [-Wunused-variable]
lld n,val;
^
1 warning generated.You do compile with warnings enabled, right?
-
#include and #include are superfluous. (Put the remaining ones in alphabetical order.)-
using namespace std; is superfluous, and even if you used anything in the std namespace, a blanket import like that is a harmful habit.-
The
debug flag is unused.-
typedef unsigned long long int llu; is never used. Furthermore, the problem guarantees 1 ≤ n ≤ 109, so a long would be sufficient for n.As you suspected, a function call would help. Your
main() should look like this:int main() {
long n;
while ((1 == scanf("%ld", &n)) && (-1 != n)) {
puts(is_beehive_number(n) ? "Y" : "N");
}
return 0;
}That provides two very important improvements over your code:
-
You can see at a glance what the purpose and structure of your program are. The clutter within the loop is all gone, replaced by a function whose purpose is obvious. The loop is properly structured — the phony
while (true) is replaced by a useful test.-
There is now proper separation of concerns.
main() loops, reads the input cases, and prints the results. Most importantly, the is_beehive_number() is a pure calculation function that accepts a long and returns a bool.When the calculation code is in its own function rather than embedded in the loop, the flow of control can be expressed so much better!
bool is_beehive_number(long n) {
if (--n % 3 != 0) return false;
double sq = sqrt(4 * (n / 3) + 1);
if (sq != (long)sq) return false;
n = sq;
return (n & 1);
}I don't understand the mathematics behind your code; I just transformed the code mechanically.
Code Snippets
beehive.cpp:12:11: warning: unused variable 'val' [-Wunused-variable]
lld n,val;
^
1 warning generated.int main() {
long n;
while ((1 == scanf("%ld", &n)) && (-1 != n)) {
puts(is_beehive_number(n) ? "Y" : "N");
}
return 0;
}bool is_beehive_number(long n) {
if (--n % 3 != 0) return false;
double sq = sqrt(4 * (n / 3) + 1);
if (sq != (long)sq) return false;
n = sq;
return (n & 1);
}Context
StackExchange Code Review Q#51672, answer score: 32
Revisions (0)
No revisions yet.