Find the smallest amongst 3 numbers in C++ [duplicate]
Asked Answered
G

10

43

Is there any way to make this function more elegant? I'm new to C++, I don't know if there is a more standardized way to do this. Can this be turned into a loop so the number of variables isn't restricted as with my code?

float smallest(int x, int y, int z) {

  int smallest = 99999;

  if (x < smallest)
    smallest=x;
  if (y < smallest)
    smallest=y;
  if(z < smallest)
    smallest=z;

  return smallest;
}
Gehrke answered 24/2, 2012 at 1:37 Comment(4)
haha what if none of the values are smaller than 99999? ha.Neurath
@jogojapan or better than starting with INT_MAX (orstd::numeric_limits<int>::max() just start with the first number...Prochronism
I'm not sure it was a good idea to close this due to the C++ context. I'd like to see the generic discussion in the cited dup; and I'd also like to see the C++ discussion with templates and meta programming; and the C++11 discussion with constexpr. This looks like a better dup, even though its max instead of min: Most efficient way to find the greatest of three ints. It looks better because its C++ specific. But it lacks a good treatment of templates, meta programming and constexpr.Curbing
I would just set the min to be the first number. No worries of overflow or picking the biggest number.Asgard
L
101

If possible, I recommend using C++11 or newer which allows you to compute the desired result w/out implementing your own function (std::min). As already pointed out in one of the comments, you can do

T minimum(std::min({x, y, z}));

or

T minimum = std::min({x, y, z});

which stores the minimum of the variables x, y and z in the variable minimum of type T (note that x, y and z must have the same type or have to be implicitly convertible to it). Correspondingly, the same can be done to obtain a maximum: std::max({x, y, z}).

Limiter answered 30/7, 2014 at 13:37 Comment(5)
This should be the accepted answer. This is an example of how C++11 makes so many things so much easier and people should use it unless they have an extremely good reason. Look how much more complex and unnecessary all the other answers are, despite being written in 2012 when this superior syntax already existed.Centime
I tried int x=1;int y=2; int z=3; std::cout << std::max({x,y,z}); but it gives an compiling error. What's the correct way to do that?Paez
You could try the flag -std=c++11 on your compiler.Wilford
You might also need to #include <algorithm>, see https://mcmap.net/q/390138/-error-when-using-std-min-quot-no-matching-function-for-call-to-min-lt-brace-enclosed-initializer-list-gt-quot/150884Rath
for anyone using MSVC, don't forget to add #define NOMINMAX. https://mcmap.net/q/352654/-std-max-expected-an-identifierQuillan
K
51

There's a number of improvements that can be made.

You could use standard functions to make it clearer:

// Notice I made the return type an int instead of a float, 
// since you're passing in ints
int smallest(int x, int y, int z){
    return std::min(std::min(x, y), z);
}

Or better still, as pointed out in the comments:

int smallest(int x, int y, int z){
    return std::min({x, y, z});
}

If you want it to operate on any number of ints, you could do something like this:

int smallest(const std::vector<int>& intvec){
    int smallest = std::numeric_limits<int>::max(); // Largest possible integer
    // there are a number of ways to structure this loop, this is just one
    for (int i = 0; i < intvec.size(); ++i) 
    {
        smallest = std::min(smallest, intvec[i]);
    }
    return smallest;
}

You could also make it generic so that it'll operate on any type, instead of just ints

template <typename T>
T smallest(const std::vector<T>& vec){
    T smallest = std::numeric_limits<T>::max(); // Largest possible integer
    // there are a number of ways to structure this loop, this is just one
    for (int i = 0; i < vec.size(); ++i) 
    {
        smallest = std::min(smallest, vec[i]);
    }
    return smallest;
}
Kelter answered 24/2, 2012 at 1:42 Comment(8)
With C++11, std::min({x, y, z});.Ellamaeellan
Third example could be written using iterators so it would also work on other containers, like lists or sets.Sociopath
@Correa: I considered that, but didn't want to go too far off the deep end with neat features since I think it might just serve to confuse him.Kelter
I have three words for you: std::min_element.Seise
Please do not use the numeric limits stuff here. Simply set smallest to the first element and check starting with the second element.Seafaring
@Tim: Why? An opinion without reasoning is worthless if the reason isn't clear. Both options have pitfalls.Lauraine
@JesseGood I wish you had posted that as an answer as it is by far the best solution in this entire thread. I have no idea why Alex thought it needs to be wrapped in a non-variadic function, thus defeating the point.Centime
@underscore_d: the question was about how to make their function more elegant. I thought it would be more instructive to show a bunch of different ways to write it so they could see the different tradeoffs. They didn't ask "which function should I replace this function with".Kelter
H
12

apart min, that let you write return min(x, min(y, z)) there is ternary operator:

float smallest(int x, int y, int z){
  return x < y ? (x < z ? x : z) : (y < z ? y : z);
}
Handtohand answered 24/2, 2012 at 1:44 Comment(0)
T
5

There is a proposal to include this into the C++ library under N2485. The proposal is simple, so I've included the meaningful code below. Obviously this assumes variadic templates.

template < typename T >
const T & min ( const T & a )
{ return a ; }

template < typename T , typename ... Args >
const T & min ( const T & a , const T & b , const Args &... args )
{ return std :: min ( b < a ? b : a , args ...); }
Thermobarometer answered 24/2, 2012 at 1:50 Comment(0)
S
5
smallest=(x<((y<z)?y:z))?x:((y<z)?y:z);

Suppose,

x is one;
y is two;
z is three;

smallest = (one < ((two < three) ? two:three)) ? one:((two < three) ? two:three)
Stoltzfus answered 24/2, 2012 at 1:58 Comment(1)
Somehow in your first line there slipped a 't' in at the second closing parantheses.Gaines
V
4

A small modification

 int smallest(int x, int y, int z){
    int smallest = min(x,y);
    return min(smallest,z);
    }
Voiced answered 24/2, 2012 at 1:41 Comment(0)
G
2

In your version, you're finding the smallest value only if it's smaller than 99999.

You should compare all three values together. Also, you're getting int but returning float. Either, you should decide which kind of values you want to process, or you could create a generalized version that works with any kind that can be compared:

#include <algorithm>

template<class T>
T smallest(T x, T y, T z)
{
  return std::min(x, std::min(y, z));
}

EDIT:

Two ways to improve the code into something that operates on a vector:

#include <cstdio>
#include <algorithm>
#include <vector>

// Use a built-in function to retrieve the smallest value automatically
template<class T>
T smallest1(const std::vector<T> &values)
{
  return *std::min_element(values.begin(), values.end());
}

// Go through the vector manually
template<class T>
T smallest2(const std::vector<T> &values)
{
  // Get the first value, to make sure we're comparing with an actual value
  T best_so_far = values.front();
  // For all the other values in the vector ...
  for(unsigned i = 1; i < values.size(); ++i) {
    // ... replace if the new one is better
    if(values[i] < best_so_far)
      best_so_far = values[i];
  }
  return best_so_far;
}

int main()
{
  // Try out the code with a small vector
  std::vector<int> test;
  test.push_back(6);
  test.push_back(5);
  test.push_back(7);

  printf("%d\n", smallest1(test));
  printf("%d\n", smallest2(test));

  return 0;
}
Gerhart answered 24/2, 2012 at 1:49 Comment(0)
G
2

1) Simple Solution:

int smallest(int x, int y, int z)
{
    return std::min(std::min(x, y), z);
}

2) Better Solution (in terms of optimization):

float smallest(int x, int y, int z)
{
  return x < y ? (x < z ? x : z) : (y < z ? y : z);
}

3) your solution Modified(Simple but not efficient):

int smallest(int x, int y, int z)
{

  int smallest = x;

  if (y < smallest)
     smallest=y;
  if(z < smallest)
     smallest=z;
  return smallest;
}

4) Any number of Numbers:

For n numbers, store it in an array (array[n]), Sort the array and take the array[0] to get smallest.

    //sort the elements in ascending order
    for(int i=0;i<n;i++)
    {
      if(array[i]>array[i+1])
      {
        int temp = array[i];
        array[i] = array[i+1];
        array[i+1] = temp;
      }
    }

    //display smallesst and largest
    cout<<"Smallest: "<<array[0];
    cout<<"Largest: "<<array[n-1];   //not needed in your case
    }
Guarantor answered 24/2, 2012 at 6:51 Comment(0)
B
1

Or you can just use define, to create a macro function.

#define min(x,y,z) (x < y ? (x < z ? x : z) : (y < z ? y : z))
Brigettebrigg answered 7/3, 2013 at 6:17 Comment(2)
Avoid macros in C++ when you can: #4716331Salesgirl
Macros expansion are very confusing sometimes, try to avoid it.Overdue
H
1

You can store them in a vector and use std::min_element on that.

For example:

vector<int> values;
values.push_back(10);values.push_back(1);values.push_back(12);

int min = *std::min_element(values.begin(),values.end());
Hysterical answered 31/7, 2015 at 10:20 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.