On Some Iterative Improvements to the Code
There are at least a couple of fundamental problems in the OP posted code.
int t = i
inside the loop shadows the t
defined outside of the loop, and this inner t
is lost when the loop is exited. At this point, the value of t
in the expression -t / i * (n + 1) / 2)
is 1, the value that t
was initialized to in this scope. This problem can be fixed by changing int t = i
--> t = i
in the loop, utilizing the variable t
that exists outside of the loop, and allowing that variable to be changed in the loop, but still used after the loop exits.
But here we come to another problem. t
is set to the value of i
at the top of the loop, but before the loop exits i
is incremented. This means that the value of i
is greater than the value of t
when the loop exits. Thus -t / i
is zero in the final print statement! One way to fix this would be to change the final print statement from printf("%d", -t / i * (n + 1) / 2)
to printf("%d", -t / (i-1) * (n + 1) / 2)
. Note that this solution has a serious problem when i == 1
; this happens when n == 1
.
Since we are really only after the sign of -t
here, it might be more clear to use a conditional operator:
printf("%d", (-t < 0 ? -1 : 1) * (n + 1) / 2)
Now that i
is no longer needed outside of the loop, we should consider reducing the scope of i
. A good general principle is to keep the scopes of variables as small as practically possible. To do this, a for
loop seems appropriate:
int t = 1;
for (int i = 1; i <= (n / 2); i++) { // reduce scope of `i`
t = i;
if (i % 2 == 0)
t *= -1;
printf("%d", t);
printf(", %d, ", -t);
}
if (n % 2 == 1) {
printf("%d", (-t < 0 ? -1 : 1) * (n + 1) / 2);
}
Note now that there are still some significant problems with the code. An input of 1 skips the loop entirely and leads to the incorrect output -1
. A newline is never printed after the end of the series (this is just bad form), but worse, even inputs lead to a series printed with a comma and a space on the end! Formatting is important, and looking at the edge cases helps us to avoid bugs. In this case, the first number in a series of only one term is wrong. In a couple of previous (now deleted) answers the solutions partially worked, but gave incorrect results in the last number when the input was an odd number (for every second odd number). Lesson: investigate the edge cases when you code.
When the input is 1, the loop is never entered, but we only need to print the first number of the series. A simple way to handle this is just to handle that input explicitly before the loop.
if (n == 1) { // n == 1: the loop is not entered
printf("1\n"); // handle this special case
}
To handle the formatting issue, consider that each pass through the loop prints a pair of numbers. After the last pass, OP code prints one more line when input is an odd number. Instead, handle the last pass through the loop explicitly. If the loop is in its final iteration, print three numbers when input is odd, or print two numbers when input is even. Otherwise, in the typical case, print a pair of numbers with commas as before. Coding in this way allows all series code, except for the special case when input is 1
, to be brought inside of the loop, including allowing t
to be defined inside the loop (further reducing the scope of t
). Here is what that code looks like:
#include <stdio.h>
int main(void) {
int n;
printf("Enter your number: ");
scanf("%d", &n);
if (n == 1) { // n == 1: the loop is not entered
printf("1\n"); // handle this special case
}
for (int i = 1; i <= (n / 2); i++) {
int t = i; // reduced scope for `t`
if (i % 2 == 0)
t *= -1;
if (i == n / 2) { // print last numbers in the series
if (n % 2) { // n is odd: print 3 numbers
printf("%d, %d, %d\n",
t,
-t,
(-t < 0 ? -1 : 1) * (n + 1) / 2);
} else { // n is even: print two numbers
printf("%d, %d\n", t, -t);
}
} else { // print typical pair in the series
printf("%d, %d, ", t, -t);
}
}
return 0;
}
On Input Validation
OP code does not check for valid input from the user. In practice, code as if users are malicious and will provide any input to break your code. Even without malicious intent, a user may accidentally provide improper input. In the OP posted code, if a user inadvertently enters a letter, or anything other than a number, no value is stored in n
. In such a case n
has indeterminate value, and further attempts to use n
will result in undefined behavior. Note that scanf()
returns the number of successful assignments made, and checking this value is a part of any input validation (you should almost always check the result returned by any function that returns a value).
You could check this value, and abort the program if the user fails to enter a number. This is not graceful. A better solution might be to take input in a loop, checking to see if a successful assignment was made by scanf()
, and looping until a number is entered. This is more complicated than it sounds because the bad (and unassigned) input remains in the input stream after the call to scanf()
, and needs to be cleared before progress can be made. The scanf()
function is difficult to use correctly, and a better solution is to use fgets()
which reads a line of input into a buffer. After this, the buffer can be parsed with sscanf()
.
The fgets()
function returns a null pointer in the unlikely event of a read error; robust code checks for such errors. The code below places the series printing code in a function print_series()
, and takes and does very basic validation of input in main()
. If there is an input read error, the code simply prints an error message and exits. Note that the input[]
array is used as an input buffer, and it is generously sized so that normal input should be contained within it. More robust code would plan for the possibility that more input could arrive than can be held in this buffer. fgets()
will not overflow this buffer, but extra input would remain in the input stream in such a case, potentially causing problems for later I/O operations.
If no number can be parsed by sscanf()
, the input loop continues, printing another prompt and fetching a fresh line of input into the input buffer. Once a number has been assigned to max
, another loop prints all series from length 1 to length max
; this helps us to see that the series outputs are as expected for different values of N. Note that when the input is less than 1
for max
, the printing loop is not entered at all, and nothing is printed. This may not be the best way to handle such inputs.
#include <stdio.h>
#define MAX_INPUT 1000
void print_series(int n);
int main(void) {
char input[MAX_INPUT];
int max;
// get user input and do minimal validation
do {
printf("Enter longest series to print: ");
if (fgets(input, sizeof input, stdin) == NULL) {
puts("Input Failure");
return 1;
}
} while (sscanf(input, "%d", &max) != 1);
// printing loop
putchar('\n');
for (int n = 1; n <= max; n++) {
print_series(n);
}
return 0;
}
void print_series(int n) {
if (n == 1) {
printf("1\n");
}
for (int i = 1; i <= (n / 2); i++) {
int t = i;
if (i % 2 == 0)
t *= -1;
if (i == n / 2) {
if (n % 2) {
printf("%d, %d, %d\n",
t,
-t,
(-t < 0 ? -1 : 1) * (n + 1) / 2);
} else {
printf("%d, %d\n", t, -t);
}
} else {
printf("%d, %d, ", t, -t);
}
}
}
Sample runs:
$ ./a.out
Enter longest series to print: bad input
Enter longest series to print: 0
$ ./a.out
Enter longest series to print: 19
1
1, -1
1, -1, -2
1, -1, -2, 2
1, -1, -2, 2, 3
1, -1, -2, 2, 3, -3
1, -1, -2, 2, 3, -3, -4
1, -1, -2, 2, 3, -3, -4, 4
1, -1, -2, 2, 3, -3, -4, 4, 5
1, -1, -2, 2, 3, -3, -4, 4, 5, -5
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7, -7
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7, -7, -8
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7, -7, -8, 8
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7, -7, -8, 8, 9
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7, -7, -8, 8, 9, -9
1, -1, -2, 2, 3, -3, -4, 4, 5, -5, -6, 6, 7, -7, -8, 8, 9, -9, -10