made a time converter in c++ which can convert second,minutes and hours is there any efficient way for this

Question

i have made this time convertor this is my first project i am new to c++ and programming can you tell me if there is any way to write this code to be more efficient or this is code is ok. thanks for answering.

    #include <iostream>

    using namespace std;

   int main()
{
long double value;
int input ;
int input2;
char again;
do{                                                                    //loop for using programme again

cout << "Welcome to time converter" << endl; //welcome text
cout << "========================" << endl;

cout << "Select  option you want to convert" << endl;
cout << "1.Minutes" << endl;
cout << "2.Hours" << endl;      //User will select what to convert
cout << "3.Seconds" << endl;
cin >> input;

while(input <1 || input >3  ){
    cout << "Error : Select a valid option:" << flush;
    cin >> input;
}

cout << "Select what do you want to convert" << endl;
cout << "1.Minutes" << endl;
cout << "2.Hours" << endl;      //User will select in which he wants to convert
cout << "3.Seconds" << endl;
cin >> input2;

while(input2 <1 || input2 >3  ){
    cout << "Error : Select a valid option:" << flush;
    cin >> input2;
}

while(input ==1 && input2 ==1  ){
    cout << "Error : Select a valid option:" << flush;       //if user accidentally slects same option in both menu 
    cin >> input2;
}

while(input ==2 && input2 ==2 ){
    cout << "Error : Select a valid option:" << flush;
    cin >> input2;
}

while(input ==3 && input2 ==3  ){
    cout << "Error : Select a valid option:" << flush;
    cin >> input2;
}

if(input ==1 &&input2 ==2){
    cout << "Enter number of Minutes you want to convert in hour: " << flush;  
    cin >> value;
    cout << endl;
    cout << "Result:" << value/60 << " " << "hours"<< endl;

}else if(input ==1 &&input2 ==3){
    cout << "Enter number of minutes you want to convert in seconds: " << flush; 2
    cin >> value;
    cout << endl;
    cout << "Result:" << value*60 << endl;

}else if(input ==2 &&input2 ==1){
    cout << "Enter number of hours you want to convert in minutes:  " << flush;
    cin >> value;
    cout << endl;
    cout << "Result:" << value*60 << " "<< "minutes" << endl;

}else if(input ==2 &&input2 ==3){
    cout << "Enter number of hours you want to convert in seconds:  " << flush;
    cin >> value;
    cout << endl;
    cout << "Result:" <<  value*3600 << " "<< "seconds" << endl;

}else if(input ==3 &&input2 ==1){
    cout << "Enter number of seconds you want to convert in minutes:  " << flush;
    cin >> value;
    cout << endl;
    cout << "Result:" << value/60 << " "<< "minutes" << endl;

}else if(input ==3 &&input2 ==2){
    cout << "Enter number of seconds you want to convert in hours:  " << flush;
    cin >> value;
    cout << endl;
    cout << "Result:" << value/3600 << " "<< "hours" << endl;

}
    cout << endl;
    cout << "Press Y to use the programme " << endl;
    cout << "Press N to exit" << endl;
    cin>> again;

}while( again =='y'|| again =='Y');

cout << "Programme ended" <<endl;



return 0;

}

Answer 1

Don't repeat yourself

A major issue with your code is that you are repeating similar code over and over. Whenever you see that you are doing the same thing twice or more times, find some way to reduce it. For example, instead of checking whether the user wants to convert seconds to seconds, then checking whether they want to convert minutes to minutes, and so on, just check whether they want to convert something to the same thing:

while (input == input2) {
    std::cout << "Error: ...\n";
    ...
}

But more importantly, you'll notice that because you have 3 different time units you can convert from and to, you needed 3 * 2 = 6 blocks of code that all say "Enter number of X you want to convert in Y", read in a value, and then do the calculation and print the result. Now imagine you have 10 different time units, because you also want to support days, weeks, months, and so on. Suddenly you need to write 10 * 9 = 90 blocks of code. That's not very efficient! So first, what is different between the time units? It's the name of the unit, and some kind of multiplier. Let's build a table that holds names and the number of seconds each time unit represents:

#include <vector>

struct time_unit {
    const char *name;
    int seconds;
};

std::vector<time_unit> time_units = {
    {"seconds", 1},
    {"minutes", 60},
    {"hours", 3600},
    {"days", 86400},
    ...
};

Now you can use this to write generic code once, that uses the table to handle all the different time units. For example, just printing the list of time units becomes:

std::cout << "Select what time unit you want to convert from:\n";

for (size_t i = 0; i < time_units.size(); ++i) {
    std::cout << i << ". " << time_units[i].name << "\n";
}

Once you have the two choices in input and input2 and verified they are valid, you can then write code that converts from one to the other and prints the results:

std::cout << "Enter the number of " << time_units[input].name
          << " you want to convert to " << time_units[input2].name << ":\n";

std::cin >> value;

std::cout << "Result: " << value * time_units[input].seconds / time_units[input2].seconds << "\n";

The above example code starts numbering choices from zero. That's easier for C++, but if you want you can add '+ 1' and '- 1' where necessary to make the user's choices start from 1.

Fix your checks for invalid input

An issue with your error checking code is that you check for some property to be valid in one loop, and then follow that up with another loop to check for another property. However, the second loop no longer checks whether the first property is correct. You should have a single loop that checks whether everything is valid. I suggest you use an infinite loop that calls break if everything is correct. For example:

while(true) {
    std::cout << "Select what do you want to convert to:\n";
    std::cin >> input2;

    if (input2 >= 0 && input2 < time_units.size()) {
        std::cout << "Error: not a valid choice.\n";
        continue;
    }

    if (input2 == input) {
        std::cout << "Error: you need to select a different unit than the one you convert from.\n";
        continue;
    }

    break;
}

Don't add unnecessary restrictions

While it is good that you check whether input values are valid integers in the desired range, there is no harm in allowing the user convert from seconds to seconds. Sure, it is useless, but on the other hand it should just work, and allowing this actually reduces the amount of code you have to write.