Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Return index of smallest element in array [closed]

I'm trying to return the index with the smallest element in an array of integers. Am I missing something? After I put my integers in, it doesn't return the index.

UPDATE: I get an error at the end of int main() about the array stack being corrupted. Thank you. My code is as follows:

#include <iostream>
#include <conio.h>

using namespace std;

int indexofSmallestElement(double array[], int size);

int main()
{    
int size = 10; 
double array[10];

for (int i = 0; i <= size; i++)
{
    cout << "Enter an integer: " << endl;
    cin >> array[i];
}

indexofSmallestElement(array, size);
}

int indexofSmallestElement(double array[], int size)
{
int index = 0;

if (size != 1)
{

    int n = array[0];
    for (int i = 1; i < size; i++)
    {
        if (array[i] < n)
        {
            n = array[i];
            index = i;
        }
    }
}
return index;
}
like image 801
gosutag Avatar asked Nov 26 '12 02:11

gosutag


2 Answers

A number of people have shown you their variant of indexofSmallestElement. I will include mine along with an explanation of why I think it's better:

int indexofSmallestElement(double array[], int size)
{
    int index = 0;

    for(int i = 1; i < size; i++)
    {
        if(array[i] < array[index])
            index = i;              
    }

    return index;
}

You will notice that I do away with the n variable, which you used to hold the smallest value encountered so far. I did this because, in my experience, having to keep two different things synchronized can be a source of subtle bugs. Even in this simple case, it was the source of multiple bugs, not the least of which was that your n was declared an int but you were assigning values of type double in it.

Bottom line: do away with the n variable and just keep track of one thing: the index.


Update: of course, it goes without saying that with C++17 the only truly acceptable answer for this question is to use std::min_element:

#include <iostream>
#include <algorithm>
#include <iterator>
#include <cstdint>
#include <array>
#include <vector>

template <typename Iter>
auto indexofSmallestElement(Iter first, Iter last)
{
    return std::distance(first,
        std::min_element(first, last));
}

template <typename T, std::size_t N>
auto indexofSmallestElement(std::array<T, N> arr)
{
    return std::distance(std::begin(arr),
        std::min_element(std::begin(arr), std::end(arr)));
}

template <typename T>
auto indexofSmallestElement(std::vector<T> vec)
{
    return std::distance(std::begin(vec),
        std::min_element(std::begin(vec), std::end(vec)));
}


int main(int, char **)
{
    int arr[10] = { 7, 3, 4, 2, 0, 1, 9, 5, 6, 8 };

    auto x = indexofSmallestElement(std::begin(arr), std::end(arr));

    std::cout
        << "The smallest element in 'arr' is at index "
        << x << ": " << arr[x] << "\n";

    std::array<float, 5> fa { 0.0, 2.1, -1.7, 3.3, -4.2 };

    auto y = indexofSmallestElement(fa);

    std::cout
        << "The smallest element in 'fa' is at index "
        << y << ": " << fa[y] << "\n";

    return 0;
}
like image 157
Nik Bougalis Avatar answered Sep 28 '22 10:09

Nik Bougalis


It should be n = array[0] instead of array[0] = n. It means you are supposing first element of the array to be the smallest in the beginning and then comparing it with others.

Moreover in your loop, you are exceeding the bound of your array. The for loop should run till i < size and not i <= size.

Your code should be like this..

int indexofSmallestElement(double array[], int size)
{
  int index = 0 ;
  double n = array[0] ;
  for (int i = 1; i < size; ++i)
  {
    if (array[i] < n)
    {
        n = array[i] ;
        index = i ;
    }
  }
 return index;
}
like image 31
Coding Mash Avatar answered Sep 28 '22 11:09

Coding Mash