Beginner to OOP: Do these classes properly represent the objects in Java?

Question

These classes are part of a program I am working on to convert Arabic numerals to Roman numerals and also the reverse. Do these classes look correct for representing objects in Java?

public class ArabicNumeral {
private final int value; 

public ArabicNumeral(final int value) {
    if (value < 0) {
        throw new NumberFormatException("Numbers must be positive");
    } 
    this.value = value;
}

public int getThousands() {
    return getCardinalValueFor(1000);
}

public int getHundreds() {
    return getCardinalValueFor(100);
}

public int getTens() {
    return getCardinalValueFor(10);
}

public int getOnes() {
    return getCardinalValueFor(1);
}

private int getCardinalValueFor(final int divisor) {
    return value / divisor % 10;
}
}



public enum RomanNumerals { 
I("I", 1),
V("V", 5),
X("X", 10),
L("L", 50),
C("C", 100),
D("D", 500),    
M("M", 1000);

private final String symbol;
private final int value;

RomanNumerals(final String symbol, final int value) {
    this.symbol = symbol;
    this.value = value;
}

String symbol() {
    return symbol;
}

int value() {
    return value;
}

public RomanNumerals getMultipleOfFive() {
    return this.ordinal() < RomanNumerals.values().length - 1
            ? RomanNumerals.values()[this.ordinal() + 1] : null;
}

public RomanNumerals getMultipleOfTen() {
    return this.ordinal() < RomanNumerals.values().length - 1
            ? RomanNumerals.values()[this.ordinal() + 2] : null;
}
}

Answer 1

I wouldn't do an ArabicNumeral, too, as this is the default numeric system of java.

From a design view, I would see an object RomanNumeral, which is an aggregation of RomanLiteralobjects, this RomanLiteral, are the constant roman string->decimal value pairs, which can't be computed.

Both classes can extend java.lang.Number

Class RomanLiteral represents one base value of the roman numeral system, it is a String - Integer relation, e.g. "V" -> 5. toString() is overriden to return the String value.

Class RomanNumeral Represents a fully roman numeral, it is an aggregate of RomanLiterals, which are basic String - Integer relation, e.g. "V" -> 5, and so on.

Attributes

• defaultRomanLiteralSet this Set containts the basic constant RomanLiterals
• literals this is the List which contains all RomanLiterals that aggregate this RomanNumeral

Methods

• RomanNumeral parseRomanNumeral(Integer) is an analogy to Integer#parseInt(String) this method may be static, and returns the RomanNumeral of the Integer param.
• add(RomanNumeral) adds an RomanNumeral to the current value of the object and returns it.
• add(RomanLiteral) adds the RomanLiteral to the current value of the object and returns it.
• toString() is overriden to return the right String representation of the roman numeral - there happens some calculations with the matching values in the default-set. Here actually gets the roman String calculated out of the integer value of the aggregate.

I think that would be a good useable design. I would stay quite near to the Number style of java, and would do my implementation and design in this direction. (analgous methods and so on..)

Answer 2

• RomanNumerals.D.getMultipleOfTen() is java.lang.ArrayIndexOutOfBoundsException
• RomanNumerals.D.getMultipleOfFive() is M

What does getMultipleOfFive mean? And why should it return 1000 when called on 500?

• symbol() and value() should be getSymbol() and getValue() by convention.

• symbol() and value() why are they package-private? What are other classes in the package that use them?

  • Where does the conversion take place?

Objects, like everything else, are meaningful only in a context. There is not a one-correct-representation of an arabic numeral. Arabic numerals are default representation so you probably do not need ArabicNumeral at all. only logic there is in:

private int getCardinalValueFor(final int divisor) {
    return value / divisor % 10;
}

which can be rewritten as :

public static int getCardinalValueFor(int value, int divisor) {
    return value / divisor % 10;
}

Answer 3

This is about general design structure.

You have methods getThousands(), getHundreds() etc those are calling getCardinalValueFor() with different parameters. You don't need so many methods just to provide different parameters.

As per Effective Java Item number #40 (Design method signatures carefully),

Every method should “pull its weight.” Too many methods make a class difficult to learn, use,document, test, and maintain.