b1GZZ
b1GZZ

Reputation: 31

What's wrong in my if statements logic?

In this little program I try to order 3 numbers in a descending order. But seems like the line in which has "// 3 2 1 - doesn't work" as a comment isn't working as expected. It seems like my logic is correct.

My input: 4, 554 and 454545

Output: (which is not what I wanted) 554, 454545 and 4

If the value hold on the integer numbThree is bigger than numbOne and if numbOne is NOT bigger than numbTwo (NOT == else) it should ouput numbThree, numbTwo and numbOne in this order, why doesn't it work?

#include <iostream>

int main() {
int numbOne = 0, numbTwo = 0, numbThree = 0;
std::cin >> numbOne >> numbTwo >> numbThree;

if (numbOne > numbTwo) {
    if (numbTwo > numbThree) {
        std::cout << numbOne << " " << numbTwo << " " << numbThree << std::endl; // 1 2 3
    }
    else {
        std::cout << numbOne << " " << numbThree << " " << numbTwo<< std::endl; // 1 3 2
    }
}
else if (numbTwo > numbOne) {
    if (numbOne > numbThree) {
        std::cout << numbTwo << " " << numbOne << " " << numbThree << std::endl; // 2 1 3 - works
    }
    else {
        std::cout << numbTwo << " " << numbThree << " " << numbOne << std::endl; // 2 3 1
    }
}
else if (numbThree > numbOne) {
    if (numbOne > numbTwo) {
        std::cout << numbThree << " " << numbOne << " " << numbTwo << std::endl; // 3 1 2
    }
    else {
        std::cout << numbThree << " " << numbTwo << " " << numbOne << std::endl; // 3 2 1 - doesn't work
    }
}

std::cin.get();
std::cin.ignore();
return 0;
}

Thanks in advance for helping me out.

Upvotes: 1

Views: 113

Answers (2)

Walter
Walter

Reputation: 45484

You cannot, in general, sort 3 numbers with 2 comparisons (see YSC's comment for a hard reason in terms of information content). Already your case 1 3 2 is flawed: what if numbThree > numbOne?

In general you have to allow for up to 3 comparisons. Of course, you can simply use the sort functionality provided by the standard library (i.e. by the language). If you don't want to (for some reason), then the correct logic (for ascending order) is

if(a<b)
  if     (b<c) // a,b,c   // 2 comparisons
  else if(a<c) // a,c,b   // 3 comparisons
  else         // c,a,b   // 3 comparisons
else
  if(    (a<c) // b,a,c   // 2 comparisons
  else if(b<c) // b,c,a   // 3 comparisons
  else         // c,b,a   // 3 comparisons

Thus, in 4 out of 6 possible cases we need 3 rather than 2 comparisons.

Upvotes: 8

stefaanv
stefaanv

Reputation: 14392

Not intended as an answer, but as an illustration of the comment by Sam Varshavchik:

What's wrong is that the code should use a small array, and std::sort, instead of this kind of spaghetti code.

While Sam is right about production code, as an exercise of how to implement the logic, the question is okay and there is already a solution.

#include <iostream>
#include <vector>
#include <algorithm>

int main()
{
    std::vector<int> v(3);
    if (! (std::cin >> v[0] >> v[1] >> v[2])) { exit(-1); }
    std::sort(v.begin(), v.end(), std::greater<int>());
    for (auto c: v) { std::cout << c << " "; }
    std::cout << "\n";
}

Upvotes: 1

Related Questions