Roman numerals to decimal

This is a program for converting Roman numerals to their decimal equivalents. There must be a better way of distinguishing between, for example, 'IV' and 'VI', than what I have currently written.

#include 
#include 
using namespace std;

class romanType{
public:
    romanType();
    romanType(string s);
    ~romanType();

        void setRoman(string); //Set the Roman numeral from user entries.
        int romanToDecimal(); //Convert the Roman numeral(string) to Decimal value.
        void printDecimal(); //Display the decimal value.
        void printRoman(); //Display the Roman numeral value.
private:
    string romanNum;
    int decimalNum = 0;

};

romanType::romanType()
{
    romanNum = 1;
}

romanType::~romanType()
{
}

void romanType::setRoman(string troll)
{
    romanNum = troll;
}

int romanType::romanToDecimal()
{
    for (int i = 0; i  0 && romanNum[i - 1] == 'I')
                decimalNum -= 2;
            decimalNum += 5;
        }
        if (romanNum[i] == 'X')
        {
            if (i > 0 && romanNum[i - 1] == 'I')
                decimalNum -= 2;
            decimalNum += 10;
        }
        if (romanNum[i] == 'L')
        {
            if (i > 0 && romanNum[i - 1] == 'X')
                decimalNum -= 20;
            decimalNum += 50;
        }
    }
    cout > numerals;

        romanType Rom;
        Rom.setRoman(numerals);
        Rom.romanToDecimal();

    } while (true);
    system("PAUSE");

}

Sample output:

Enter roman numerals: IX
9
Enter roman numerals: XI
11
Enter roman numerals: XIX
19
Enter roman numerals: XX
20
Enter roman numerals: LVIII
58
Enter roman numerals: LXXXVIII
88
Enter roman numerals: LXVI
66
Enter roman numerals: LXIV
64
Enter roman numerals:


-
Try not to use using namespace std and system("PAUSE").

The former is okay for shorter programs or toy programs, or when used in local scope as opposed to global (such as within a function).

The latter is best avoided as it is problematic while safer and more portable alternatives are in existence, such as std::cin.get(). It's also non-portable and only works with Windows.

-
In certain environments (or depending on personal preference), new user-defined types should be capitalized and objects should be lowercase.

NewType objOfNewType;

-
Prefer an initializer list here:

NewType::NewType() : dataMember1(/* value */) {}

Although this may not make a huge difference in your code, it's still good to know about it when writing larger programs. It is best when initializing objects (especially when the arguments are of the same name) and necessary when initializing constants.

-
As you're not using the destructor, you can leave it out. The compiler will make one for you that does any of the default cleanup.

-
You shouldn't do this in the class declaration:

int decimalNum = 0;

You should instead put that into the initializer list as mentioned above:

// entries are separated by commas
// can also be listed on separate lines
className::className() : member1(0), member2(0) {}

-
You have declared printRoman(), but have not defined it. Better yet, overload operator

-
int is not a good loop counter type here:

for (int i = 0; i < romanNum.length(); i++) {}

romanNum is of type std::string, so you should use std::string::size_type. This will ensure that you can loop through any string size while not being limited by int's size.

Better yet, use its iterators instead (using a range-based for-loop in C++11):

for (auto& iter : romanNum)
{
    if (iter == 'I')
        decimalNum++;
}

Another alternative (if you don't have C++11 and cannot use the above):

for (std::string::iterator = romanNum.begin(); iter != romanNum.end(); ++iter)
{
    if (*iter == 'I')
        decimalNum++;
}

-
Prefer not to pass-by-value here:

void romanType::setRoman(string troll)
{
    romanNum = troll;
}

As you're not modifying troll, prefer to pass by constant reference (const and &). This will avoid unneeded copying, thereby also increasing performance. It's best only for non-native types (such as std::string), so disregard it for any of the native ones.

Example (in different forms):

void func(const std::string& str) {}

void func(std::string const& str) {}

-
do-while loops are usually considered less-readable than while loops. However, you don't need a while loop here as that would require adding another user input before the loop.

Instead, I'd recommend the "infinite loop" for (;;):

for (;;)
{
    std::cout << "This message will print forever!\n";
    std::cout << "But please do end this eventually!\n\n";

    bool keepGoing = true;
    if (!keepGoing) break; // or return
}

As described, this will keep looping until the loop is ended (usually with a break, return, or return X if non-void). You'll just need a conditional that will decide when the loop should end.

-
Prefer to use variables as close in scope as possible:

std::cout > input;

-
This is unneeded:

romanType Rom;
Rom.setRoman(numerals);

You're invoking the default destructor (no arguments) while calling the mutator.

Instead, the object should be constructed right away with numerals`:

romanType Rom(numerals);

Beyond that, you won't need a mutator in your program. They can also be bad for encapsulation, but that's another topic. For now, focus on proper object construction while only using necessary implementations. Keep your code as concise as possible.

Note that my previous point about the initializer list was for example purposes. In this case, you'll need to add an argument to the list.

NewType::NewType(ArgType arg) : member1(arg) {}

With this in place:

NewType objOfNewType(5); // will compile
NewType objOfNewType;    // will not compile

In your program, the latter should still happen as the conversion process shouldn't work with no input given to the object. You can also keep your existing default constructor, allowing the latter to still work. But there may be no need for a default value if you want the user to give input.

StackExchange Code Review Q#35453, answer score: 18