Currency Converter Switch Case Problem

[Bogeyman]

Hello. As a way of testing out switch, I wrote a simple currency converter. The problem I have is that when one currency is converted to another, it immediately jumps to the next currency to be converted. I have checked the syntax and I have break; in all the correct places as far as I can tell.

Here is part of the code:

#include "std_lib_facilities.h"

int main()
{
    int currency1 = 0, value = 0, currency2 = 0;
    double rate = 0;

    cout << "Currency Converter Using Switch.   *Market values accurate as of 01/08/2014*\n" << endl;

    do
    {
    cout << "Available Currencies:\n---------------------\n| (1) Euro          |\n"
         << "| (2) Pound         |\n| (3) Dollar        |\n| (4) Yen           |\n| (5) Danish Krone  |"
         << "\n| (6) Chinese Yuan  |\n---------------------\n\n";

    cout << "Currencies are chosen by entering their corresponding index value.\n\n";
    cout << "Please choose a currency: ";
    cin >> currency1;

    switch (currency1)
    {
        case 1:
            cout << "\nYou have selected Euro.\n\n"
                 << "Please choose a value in Euro: ";
            cin >> value;
            cout << "\nYou have chosen " << value << " Euro.\n\n"
                 << "Please choose the currency you wish to convert to: ";
            cin >> currency2;

        switch (currency2)
        {
            case 1:
                cout << "\nYou have chosen Euro.\n\n";
                rate = value * 1;
                cout << value << " Euro == " << rate << " Euro.\n\n\n\n";
                break;
            case 2:
                cout << "\nYou have chosen Pounds.\n\n";
                rate = value * 0.795833;
                cout << value << " Euro == " << rate << " Pounds.\n\n\n\n";
                break;
            case 3:
                cout << "\nYou have chosen Dollars.\n\n";
                rate = value * 1.33943;
                cout << value << " Euro == " << rate << " Dollars.\n\n\n\n";
                break;
            case 4:
                cout << "\nYou have chosen Yen.\n\n";
                rate = value * 137.870;
                cout << value << " Euro == " << rate << " Yen.\n\n\n\n";
                break;
            case 5:
                cout << "\nYou have chosen Danish Krone.\n\n";
                rate = value * 7.45595;
                cout << value << " Euro == " << rate << " Danish Krone.\n\n\n\n";
                break;
            case 6:
                cout << "\nYou have chosen Chinese Yuan.\n\n";
                rate = value * 8.27564;
                cout << value << " Euro == " << rate << " Chinese Yuan.\n\n\n\n";
                break;
        }

        case 2:
            cout << "\nYou have selected Pound.\n\n"
                 << "Please choose a value in Pounds: ";
            cin >> value;
            cout << "\nYou have chosen " << value << " Pounds.\n\n"
                 << "Please choose the currency you wish to convert to: ";
            cin >> currency2;

        switch (currency2)
        {
            case 1:
                cout << "\nYou have chosen Euro.\n\n";
                rate = value * 1.25657;
                cout << value << " Pounds == " << rate << " Euro.\n\n\n\n";
                break;
            case 2:
                cout << "\nYou have chosen Pounds.\n\n";
                rate = value * 1;
                cout << value << " Pounds == " << rate << " Pounds.\n\n\n\n";
                break;
            case 3:
                cout << "\nYou have chosen Dollars.\n\n";
                rate = value * 1.68296;
                cout << value << " Pounds == " << rate << " Dollars.\n\n\n\n";
                break;
            case 4:
                cout << "\nYou have chosen Yen.\n\n";
                rate = value * 173.261;
                cout << value << " Pounds == " << rate << " Yen.\n\n\n\n";
                break;
            case 5:
                cout << "\nYou have chosen Danish Krone.\n\n";
                rate = value * 9.36864;
                cout << value << " Pounds == " << rate << " Danish Krone.\n\n\n\n";
                break;
            case 6:
                cout << "\nYou have chosen Chinese Yuan.\n\n";
                rate = value * 10.4010;
                cout << value << " Pounds == " << rate << " Chinese Yuan.\n\n\n\n";
                break;
        }
    }

    }while(1 > 0);



    return 0;
}

This only shows the Euro and Pounds portion, as the rest is too long. Any help is appreciated.

[kbw]

You need a break after line 62.

Th error is not obvious because your indentation is misleading.

[xenovia12]

@Bogeyman kbw is right. I think you should also put a break; in line 29.

[Bogeyman]

Thanks guys, that did the trick.
Now however, after adding a default case, it only applies to case 1 like this:

switch (currency1)
    {
        case 1:
            cout << "\nYou have selected Euro.\n\n"
                 << "Please choose a value in Euro: ";
            cin >> value;
            cout << "\nYou have chosen " << value << " Euro.\n\n"
                 << "Please choose the currency you wish to convert to: ";
            cin >> currency2;
        default:
            cout << "\nInvalid value.\n\n\n\n";

It won't let me use any more default cases, yet the default case I added only applies to case 1, for Euro.

As well as this, I am trying to find a way to stop the program from crashing when a non numeric value is entered. Is there an easy, simple way to do this?

The last thing that I would like to know is what better ways there are of looping the code, aside from the do while loop I have implemented.

I know that's a lot, so thanks to anyone who can help me with any of that.

Edit: Actually, default doesn't work properly, even on case 1. switch is new to me, so I'm probably just making simple mistakes, but I can't figure it out.

[kbw]

You're still not using break.

[Bogeyman]

There's no need to. Using break; would stop the program from continuing after it asks for the second currency.

[AbstractionAnon]

You're missing break in all cases of your outermost switch statement.

It won't let me use any more default cases, yet the default case I added only applies to case 1, for Euro.

default applies to the switch statement as a whole, not to a specific case. i.e. if currency1 does not match any of the named cases, then the default branch is executed.

Because you have no break statement after lne 9, case 1 will always fall through to the default case.

IMO, nested switch statements are ugly and prone to error. Much better practice to isolate the nested switch statements to their own functions. This also makes the outer switch statement easier to read since the name of the function being called should be meaningful.

[Bogeyman]

@AbstractionAnon;
Thank you. The problem is that if I add a break; after line nine, the currency conversion is never processed.

If there's one thing this exercise has taught me so far, it's that if/else if/else is much easier to structure and follow. switch is just too finicky.

[AbstractionAnon]

Line 9 is the incorrect place. You didn't show the nested switch statement, so line 9 appeared to be the last executable statement in the case.

The break for cases in the outer switch belongs after the inner switch statements.

Referring to your original code, you should have a break at line 63 and after line 104. This is precisely why I said nested switch statements are ugly and error prone. Had the inner switch statements been in their own functions, the correct place for the missing break statements would have been very obvious.

[Bogeyman]

@AbstractionAnon;
I actually do have break; now implemented at the places you referred to in the code. It's still not working, so I think I'll just put this one down to experience, whilst simultaneously fostering my preference for if/else if/else over switch. As for functions, yes I do believe it's time to start learning how to use them. Otherwise, I'm probably going to end up writing some mighty unwieldy code.

My other questions still stand if anyone is willing to answer them.

Is there a simple, all purpose way of dealing with non-numeric input handling?

Which loop is best for running the same code more than once, with added control, such as "would you like to go again?", followed by a choice of yes or no. At the moment I am using what feels like a crude do while loop to make my code run indefinitely. Is there a way to add more control to your loop? (I'm trying to move away from goto).

[AbstractionAnon]

whilst simultaneously fostering my preference for if/else if/else over switch.

There's nothing wrong with switch statements when implemented correctly.
When used to select between a number of cases, switch statements are often clearer than a jumble of if/else statements. I think you will find with your current program structure with nested statements, nested if/else statements will actually get more unwieldy than switch statements.

Is there a simple, all purpose way of dealing with non-numeric input handling?

The standard way is as follows:

 
    int val; 
    cout << "Enter input:";
    do 
    {   if (! cin >> val)
        {  // input failed (not numeric)
            cin.clear ();   // reset the stream error flags
            cin.ignore (1024);   // eat any chars left in the buffer
            cout << "try again: ";
            continue;  //  next iteration of the loop
        }
        //  have good input
    } while (val < 1 || val > 6);  // or whatever the valid input range is 
    //  Have range checked value 

Which loop is best for running the same code more than once, with added control

do/while is just fine for that.

I'm trying to move away from goto

Good move.

As for functions, yes I do believe it's time to start learning how to use them

Any time you see a lot of repeated code or even code that looks very similar, that's a good indication the repeated or similar code should be moved into a function.

You should consider a table driven approach to your code.
I did this for only 3 currencies, but you should be able to extend it to any number of currencies easily simply by modifying MAX_CURRENCY and the two tables. The code does not change. No nested switch or if statements.

#include <iostream>
#include <string> 
using namespace std; 

const int MAX_CURRENCY = 3;

const string currency_name[MAX_CURRENCY] = 
{	"Euro",
	"Pound",
	"Dollar",
};

const double exchange_rate[MAX_CURRENCY][MAX_CURRENCY] = 
{	{ 1, 0.795833, 1.33943 },	// Euro->Euro, Euro->pound, euro->dollar
	{ 1.25657, 1, 1.68296 },	// Pound->Euro, Pound->Pound, Pound->Dollar
								// Don't have rates for dollar->Euro or pound
};

int main()
{
    int currency1 = 0, value = 0, currency2 = 0;
    double rate = 0;

    cout << "Currency Converter *Market values accurate as of 01/08/2014*\n" << endl;

    while (true) 
	{	cout << "Available Currencies:" << endl;
		cout << "---------------------" << endl;
		for (int i=0; i<MAX_CURRENCY; i++)
			cout << i+1 << ". " << currency_name[i] << endl;

		cout << "Currencies are chosen by entering their corresponding index value.\n\n";
		cout << "Please choose a currency: ";
		cin >> currency1;
		currency1--;  // Convert to 0 base
		cout << "You have selected " << currency_name[currency1] << endl;
		cout << "Please enter a value in " << currency_name[currency1] << endl;
		cin >> value; 
		cout << "You have chosen " << value << currency_name[currency1] << endl;
	    cout << "Please choose the currency you wish to convert to: ";
		cin >> currency2;
		currency2--;	//	Convert to 0 base
		cout << "You have chosen " << currency_name[currency2] << endl; 
		rate = value * exchange_rate[currency1][currency2];
		cout << value << " " << currency_name[currency1] 
			 << " = " << rate << currency_name[currency2] << endl;
    };
    return 0;
}

[Bogeyman]

You have been a great help. You have answered all my questions thoroughly and with clarity. It's these kinds of answers that really help learners to attain new insights into the language and its varied concepts. Thank you.