Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is my sorting question in ascending order a good solution?

#include <iostream>
using namespace std;

int main() {
// Problem #2 Ascending Order
int num1, num2, num3, small, medium, large;
cin >> num1 >> num2 >> num3;

if(num1 <= num2 && num1 <= num3) {
    small = num1;
    if(num2 <= num3) {
        medium = num2;
        large = num3;
    } else {
        large = num2;
        medium = num3;
    }
} else if(num2 <= num1 && num2 <= num3) {
    small = num2;
    if(num3 <= num1) {
        medium = num3;
        large = num1;
    } else {
        large = num3;
        medium = num1;
    }
} else {
    small = num3;
    if(num2 <= num1) {
        medium = num2;
        large = num1;
    } else {
        large = num2;
        medium = num1;
    }
}

cout << small << " " << medium << " " << large;
return 0;

/* This code is to sort 3 numbers in ascending order, this is the solution I came up with. Im not sure if its a good answer, is their any better way to do this? Should I be happy with this solution? Any thoughts? Im new to programming and want to know if the solutions Im coming up with are realisitic? Like are they good enough for me to get a job if I problem solve like this? Does this show that Im a bad programmer? */

like image 634
Woyal Avatar asked Dec 31 '22 14:12

Woyal


1 Answers

No. Your code is too complicated. Seriously, it takes me some effort to read it and to understand every detail. It is very repetitive and has lots of potential for simple mistakes ruining correctness. Thats a too high price to pay to sort three numbers.

Don't reinvent a wheel and know your <algorithm>s.

You want the smallest number (std::min), the biggest (std::max) and the other number:

#include <algorithm>
#include <iostream>

int main(){
    int a = 42;
    int b = 2;
    int c = -13;

    int smallest = std::min({a,b,c});
    int biggest = std::max({a,b,c});
    // int middle = a+b+c-smallest-biggest;      // might overflow
    int middle = a ^ b ^ c ^ smallest ^ biggest; // no overflow

    std::cout << smallest << " " << middle << " " << biggest << "\n";
}

Live Demo

Credits for pointing out the overflow and providing the non-overflow solution goes to PatrickRoberts.

Even if, eg. as an exercise, you would avoid to use standard algorithms, you should still use functions. They need not be complicated. Already a int max(int a,int b) would help to simplify your code a lot.

like image 142
463035818_is_not_a_number Avatar answered Jan 02 '23 02:01

463035818_is_not_a_number