-1

I am trying to find the smallest element of an array, I think I am doing it correctly however I am receiving 0 as my smallest element, however, I am not entering 0 for any elements of my array. I understand some things in here are done poorly but this is my whole code in order to be reproducible and fixed.

#include<stdio.h>
#include<string.h>
#include <stdlib.h>
int main(){
    char input[500];

    printf("Enter a list of whitespace-separated real numbers terminated by EOF or 'end'.");
    puts("");
    printf("-----------------------------------------------------------------------------");
    puts("");
    gets(input);

    int size = strlen(input);
    
    int elements[size];
    int i = 0;

    char *p = strtok(input," ");

    while( p != NULL)
    {
       elements[i++] = strtol(p, NULL, 10);
       p = strtok(NULL," ");
    }

    //NUM OF ELEMENTS
    int numOfElements = 0;
    for(int j = 0; j < i; j++){
        elements[j] = numOfElements++;
    }
    
    //MIN ELEMENT
    int min = INT_MAX;
    for(int k = 0; k < i; k++){
        if(elements[k] < min){
            min = elements[k];
        }
    }

    printf("-----------------------------------------------------------------------------\n");
    printf("# of Elements:          %d\n", numOfElements);
    printf("Minimum:            %d\n", min);


    return 0;
}

RESULT:

    Enter a list of whitespace-separated numbers.
-----------------------------------------------------------------------------
1 2 3 4 5
-----------------------------------------------------------------------------
# of Elements:                  5
Minimum:                        0

EXPECTED:

    Enter a list of whitespace-separated numbers.
-----------------------------------------------------------------------------
1 2 3 4 5
-----------------------------------------------------------------------------
# of Elements:                  5
Minimum:                        1
JK3
  • 35
  • 5
  • 2
    What is `i`? Needs a [mcve]. – Shawn Jun 29 '21 at 02:37
  • @Shawn its the amount of values in the array, for this case 5. Same as length of array. – JK3 Jun 29 '21 at 02:39
  • first of u don't need the else part, it's just going to assign it to the first item in element every time something bigger than min comes. could u run `printf("inside if %d", min)` inside the if after the min = element[k] to see if it goes inside? and a `printf("in loop")` in for loop to see if its working properly – Jaison Thomas Jun 29 '21 at 02:47
  • I suspect that there is something wrong with how you are filling the `elements` array. Please provide a [mre]. – Andreas Wenzel Jun 29 '21 at 02:49
  • @JaisonThomas I did that and it prints 0 still inside the loop. I have edited my post with more info incase I am filling the array wrongly. – JK3 Jun 29 '21 at 02:55
  • 2
    Just as a side note (not the reason for the problem): [Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/q/1694036/12149471) – Andreas Wenzel Jun 29 '21 at 02:59
  • 1
    Thanks for adding your actual code. You're getting a minimum of 0 because that's what you're setting the first element of the array to. For some reason you're replacing what you read from the user with the numbers 0, 1, 2, 3, etc. Lots of other strange things, too, but that's the big cause of your problem. – Shawn Jun 29 '21 at 03:00

2 Answers2

4

The problem in the original posted excerpt was the else in this loop:

int min = INT_MAX; //I tried int min = elements[0] also
for(int k = 0; k < i; k++){
    if(elements[k] < min){
        min = elements[k];
    }else{
        min = elements[0];
    }
}

Consider what happens if you're partway through the array, and you've updated min multiple times, but now you encounter an element that's >= min. The else will reset min to elements[0]. Just delete it:

int min = INT_MAX; //I tried int min = elements[0] also
for(int k = 0; k < i; k++){
    if(elements[k] < min){
        min = elements[k];
    }
}

As an aside, either initialization of min will work. If you initialize it to elements[0], then you can start the loop at k = 1.

Update: The above answer was based on the originally posted code excerpt. Now that more code has been posted (and the fix I showed above has been applied), there are additional problems.

The main problem is the numOfElements loop. This loop completely erases the values in elements, replacing them with 0, 1, 2, etc. So the minimum value really is 0. It's not clear what the point of this loop is. I suggest deleting it entirely. The number of values in elements is just i, so there's nothing to compute. You could rename i to numOfElements if you like.

Other problems: (1) The code needs to include <limits.h> for the definition of INT_MAX, and (2) It should not be using gets. Change it to use fgets or something similar.

Tom Karzes
  • 22,815
  • 2
  • 22
  • 41
  • Hey Tom, thanks for your reply. I tried taking out the else before and it had no effect, I still get 0 for the min. – JK3 Jun 29 '21 at 02:54
  • @JK3 Now that you've posted more code, I can see a second bug, and I've updated my answer to include additional bug fixes. Refresh your page and take a look and see if you can get it to work. – Tom Karzes Jun 29 '21 at 03:03
  • Got it all figured out tom, thank you for your help! – JK3 Jun 29 '21 at 03:12
  • @JK3 Good, glad you worked it out! As I said, the counting loop served no purpose, since you already knew how many elements there were (i.e., `i`). You could rename `i` to `numOfElements` if you like. – Tom Karzes Jun 29 '21 at 03:15
2
//NUM OF ELEMENTS
    int numOfElements = 0;
    for(int j = 0; j < i; j++){
        elements[j] = numOfElements++;
    }

problem in here, you reset the elements array when you get count of this array

here is gdb mess when pass here

(gdb) p min
$3 = 0
(gdb) p elem
elem-hash.h  elements     
(gdb) p elements 
$4 = {0, 1, 2, 3, 4, 5, -8096, 32767, 1431652112, 21845, 1431652512}

here is the right

//NUM OF ELEMENTS
    int numOfElements = 0;
    for(int j = 0; j < i; j++){
        numOfElements++;
    }
  • `"here is the right"` -- I wouldn't call that the "right" way, as that loop does nothing else than set `numElements` to `i`, which can be accomplished more easily without a loop, for example with the statement `numOfElements = i;`. – Andreas Wenzel Jun 29 '21 at 03:29