Search code examples
c++arraysfor-loopif-statementswap

How to swap the positions of Min and Max in an array?


Here in the code I found the min and max values of any given array . Now I want to swap their positions, and print them out. Like Min at position of Max and vice versa. How can I change their positions? I have done it wrong, I guess.

#include <iostream>

using namespace std;

int main()
{
    int array[8] = { 0, 0, 0, 0, 0, 0, 0, 0}; 
    int min = array[0]; 
    int max = array[0]; 
    int indexOfMin = 0; 
    int indexOfMax = 0; 
    int arrSize = sizeof(array)/sizeof(array[0]); 
    int temp = 0; 

    cout << "Enter an array: "; 

    int k;
    for(k = 0; k <= arrSize; k++){ 
        cin >> array[k];
    }

    for (int i = 0; i < arrSize; i++){ 
         if(array[i] >= max ){          
            max = array[i];            
            indexOfMax = i;            
        }
    }

    for (int i = 0; i < arrSize; i++){ 
        if(array[i] == min){           
            continue;
        }
        if(array[i] < min){
            min = array[i];
            indexOfMin = i;
        }
    }

    temp = min;
    min = max;
    max = temp;

    cout << array[k] << " " <<endl;

    return 0;
}

Input = 1, 5, 9, 1, 2, 9, 1, 3
Output = 9, 5, 9, 1, 2, 1, 1, 3


Solution

  • int min = array[0];
    int max = array[0];
    

    You don't know that yet. array[0] at this point of the program is 0 ... but 0 might not be an element of the array after user input.

    int indexOfMin = 0;
    int indexOfMax = 0;
    

    Indexes into and the sizes of objects in memory should be of type std::size_t (<cstddef>) because it is guaranteed that std::size_t is big enough. There is no such guarantee for int.

    int arrSize = sizeof(array) / sizeof(array[0]);
    

    Use std::size() (<iterator>) for clearer code:

    auto const arrSize{ std::size(array) };
    
    int k;
    for (k = 0; k <= arrSize; k++) {
        cin >> array[k]; 
    }
    

    Valid array indexes range from 0 to < N for an array array[N]. You access the array out of bounds. Use k < arrSize as condition. k should be of type std::size_t.

    for (int i = 0; i < arrSize; i++) {
      if (array[i] >= max) {
          max = array[i];
          indexOfMax = i;
      }
    }
    
    for (int i = 0; i < arrSize; i++) {
      if (array[i] == min) {
          continue;
      }
      if (array[i] < min) {
          min = array[i];
          indexOfMin = i;
      }
    }
    

    If you had defined int min = array[0]; and int max = array[0]; after user input you could start these loops with i = 1. The if (array[i] == min) { continue; } buys you nothing. In contrary it wastes time with an additional comparison. Also, both loops can be combined into one:

    int min{ array[0] };
    int max{ array[0] };
    
    std::size_t indexOfMin{ 0 };
    std::size_t indexOfMax{ 0 };
    
    for (size_t i{ 1 }; i < arrSize; ++i) {
        if(array[i] < min) {
            min = array[i];
            indexOfMin = i;
        }
        else if(array[i] > max) {
            max = array[i];
            indexOfMax = i;
        }
    }
    
    temp = min;
    min = max;
    max = temp;
    

    Will swap the values of the variables min and max. Also, if swapping the minimal and maximal values in the array could be done that way, why remember their position? Try

    temp = array[indexOfMin];
    array[indexOfMax] = array[indexOfMin];
    array[indexOfMin = temp];
    

    So at the end i just write

    for (k = 0; k <= 7; k++) {
        cout << array[k] << " " << endl;
    }
    

    ?

    No, you write

    for (std::size_t k = 0; k < arrSize; k++) {
        std::cout << array[k] << " ";
    }
    std::cout.put('\n');
    

    because you (should) have declared the previous k from the input loop inside the for-loop and you make it a good habit to declare and define variables as close to where they're used as possible. Also, since you want a list in one line don't use std::endl inside the loop but print a '\n' afterwards.