I want to make an app that determines whether the given input is a positive number, and then prints them out if there are more than 2 positive numbers, but there is a weird output which I've been trying to fix for a few hours. Note : I'm somewhat of a beginner in C.
#include <stdio.h>
int main() {
const char arr[][20] = {"First Number : ", "Second Number : ", "Third Number : "};
int numbers[2]; // number list
int posNumbers[2]; // positive number list
int n = 0; // n for itinerating posNUmbers
for ( int i = 0 ; i <= 2; i ) {
printf("%s", arr[i]);
scanf("%i", &numbers[i]);
} // puts input in array
for ( int i = 0 ; i <= sizeof(numbers) / sizeof(numbers[0]) 1 ; i ) {
if (numbers[i] > 0) {
posNumbers[n 1] = numbers[i];
}
} // adds positive in array
if (sizeof(posNumbers) / sizeof(posNumbers[0]) 1 > 1) {
printf("There are atleast 2 pos numbers, which are : \n");
for ( int i = 0; i <= sizeof(posNumbers) / sizeof(posNumbers[0]) 1 ; i ) {
printf("%i", posNumbers[i]);
}
} else {
printf("THere are not enough positive numbers.");
}
}
Output : There are atleast 2 pos numbers, which are : 4419368778941968054388
CodePudding user response:
You have several misconceptions about arrays and the use of sizeof
that cause you to attempt to access values beyond the end of your numbers
and posNumbers
arrays that invokes Undefined Behavior1 in your code. You further invoke Undefined Behavior when you attempt to read from uninitialized elements in the posNumbers
array.
In C, arrays are:
- zero based. Meaning that an array of
nelem
elements has valid indexes of0 <= index < nelem
, - when looping over all elements in an array, you loop from
i = 0; i < nelem; i
, - if you fail to initialize your arrays all zero and do not fill all elements and attempt to loop over all elements in your array, you invoke Undefined Behavior when you attempt to access the uninitialized element (lesson -- initialize all arrays to begin with),
- when you use
sizeof array / sizeof array[0]
, you get the total number of elements in the array, not the number of elements filled, - your loop over
posNumbers
ignores the fact that less than all elements can be filled invoking Undefined Behavior if you attempt to read from an uninitialized element of the array, - when filling less than all values in an array, simply keep a counter to track the number of elements filled, you have
n
declared already.
Additional points to consider:
- avoid using MagicNumbers in your code and instead
#define
a constant. This helps avoid the problems you are having with2
, - above all learn you cannot use
scanf()
(or any user input function) correctly unless you check the return, especially where numeric conversions are involved. What if the user enters"four"
instead of4
?
Putting it altogether, and making a few additional changes to your output calls, you can rewrite your code to avoid the problems above as:
#include <stdio.h>
#define NVALS 3 /* if you need a constant, #define one (or more) */
int main (void) {
/* an array of pointers to the string-literals is fine */
const char *arr[] = { "First Number : ",
"Second Number : ",
"Third Number : " };
int numbers[NVALS] = {0}, /* initialize all arrays */
posNumbers[NVALS] = {0},
n = 0;
/* loop NVALS times reading number input */
for (int i = 0 ; i < NVALS; i ) {
/* no conversion involved, fputs is fine for end-of-line control */
fputs (arr[i], stdout);
/* you can't use scanf correctly unless you CHECK THE RETURN
* to validate each conversion was successfully completed
*/
if (scanf("%i", &numbers[i]) != 1) {
fputs ("error: invalid integer input.\n", stderr);
return 1;
}
/* compare if numbers[i] positive */
if (numbers[i] > 0) {
posNumbers[n ] = numbers[i];
}
}
if (n > 1) { /* check if at least two numbers positive */
puts ("\nThere are atleast 2 pos numbers, which are :");
for (int i = 0; i < n; i ) {
printf ("%i\n", posNumbers[i]);
}
}
else {
puts ("\nMust have at least two positive numbers.");
}
}
(note: a good compiler will convert printf ("const string");
to fputs ("const string", stdout);
for you, but choosing fputs()
over the variadic printf()
when there are no conversions involved indicates you understand how to choose the proper tool for the job on your own)
Example Use/Output
If less than two positive integers:
$ ./bin/threeposnumbers
First Number : -10
Second Number : 0
Third Number : 4
Must have at least two positive numbers.
If the two positive numbers are provided:
$ /bin/threeposnumbers
First Number : -10
Second Number : 2
Third Number : 4
There are atleast 2 pos numbers, which are :
2
4
If all positive numbers are provided:
$ ./bin/threeposnumbers
First Number : 10
Second Number : 2
Third Number : 4
There are atleast 2 pos numbers, which are :
10
2
4
(if you are old enough, you will recall the special significance of those numbers for a specific soft-drink marketing campaign, hint: initials DP :)
Handling non-integer input:
$ ./bin/threeposnumbers
First Number : -10
Second Number : two
error: invalid integer input.
Let me know if you have further questions.
footnotes:
1.) See:
- Undefined, unspecified and implementation-defined behavior and
- What is indeterminate behavior in C ? How is it different from undefined behavior? and
- Undefined behavior
CodePudding user response:
You say you want to store 3 numbers but you are declaring your array size to be 2. If you want to store 3 numbers this code:
int numbers[2]; // number list
int posNumbers[2]; // positive number list
Should be
int numbers[3]; // number list
int posNumbers[3]; // positive number list
You are supposed to declare the array sizes as how big as you want them but you can only use up to size - 1
since indexes start from 0.
Also you should probably fix the things that @WeatherVane is stating in the comments.
CodePudding user response:
Addendum on the answers: you can interleave then in a struct
to make it clearer which variables go together. This can also reduce the number of times you repeat yourself.
You have: a) a fixed number of numbers to enter, and b) you are going to copy all the positive numbers to a second, possibly smaller, array. The simplest is to make an equal-size array and explicitly have the size. I would refactor the code as such,
#include <stdio.h>
int main(void) {
struct {
const char *title;
int number;
} arr[] = { { "First", 0 }, { "Second", 0 }, { "Third", 0 } };
struct {
size_t size;
int number[sizeof arr / sizeof *arr];
} positives;
const int arr_size = sizeof arr / sizeof arr[0];
// enter numbers
for ( size_t i = 0 ; i < arr_size; i ) {
printf("%s Number : ", arr[i].title);
if(scanf("%i", &arr[i].number) != 1) return 1;
}
// adds positive in array
positives.size = 0;
for ( size_t i = 0 ; i < arr_size ; i ) {
if (arr[i].number > 0) {
// bounded by arr_size
positives.number[positives.size ] = arr[i].number;
}
}
if (positives.size >= 2) {
printf("There are atleast 2 pos numbers, which are : \n");
for ( size_t i = 0; i < positives.size ; i ) {
printf("%i\n", positives.number[i]);
}
} else {
printf("THere are not enough positive numbers.\n");
}
}