# Number to Roman Numeral

var romanArray = [];
var toRoman = {

analyze: function(number){
romanArray = [];
if (number >= 1000) {
return this.thousands(number);
}else if (number >= 100){
return this.hundreds(number);
}else if (number >= 10) {
return this.tens(number);
}else{
return this.last_number(number);
}
},

thousands: function(number){
var remainder = number % 1000;
var thousands = Math.floor(number / 1000);
for(var e = 0; e < thousands; e++) {
romanArray.push('M');
}
return this.hundreds(remainder);
},

hundreds: function(number){
var remainder = number % 100;
var hundreds = Math.floor(number / 100);
if (hundreds === 4){
romanArray.push('CD');
}else if(hundreds === 9){
romanArray.push('CM');
}else if(hundreds >= 5 && hundreds < 9){
romanArray.push('D');
for(var i = 0; i < (hundreds % 5); i++) {
romanArray.push('C');
}
}else if (hundreds > 0 && hundreds < 4){
for(var e = 0; e < hundreds; e++) {
romanArray.push('C');
}
}else{

}
return this.tens(remainder);
},

tens: function(number){
var remainder = number % 10;
var tens = Math.floor(number / 10);
if (tens === 4){
romanArray.push('XL');
}else if(tens === 9){
romanArray.push('XC');
}else if(tens >= 5 && tens < 9){
romanArray.push('L');
for(var i = 0; i < (tens % 5); i++) {
romanArray.push('X');
}
}else if (tens > 0 && tens < 4){
for(var e = 0; e < tens; e++) {
romanArray.push('X');
}
}else{

}
return this.last_number(remainder);
},

last_number: function (number){
if (number === 4){
romanArray.push('IV');
}else if(number === 9){
romanArray.push('IX');
}else if(number >= 5 && number < 9){
romanArray.push('V');
var remainder = number % 5;
for(var i = 0; i < remainder; i++) {
romanArray.push('I');
}
}else if (number > 0 && number < 4){
for(var e = 0; e < number; e++) {
romanArray.push('I');
}
}else{

}
return romanArray.join('');
}
};

console.log(toRoman.analyze(1000));
console.log(toRoman.analyze(2999));
console.log(toRoman.analyze(2555));


I'm wondering if this JavaScript can be refactored using some sort of base/template function.
For Example:

base_function: function (number, four, nine, five, one){
if (number === 4){
romanArray.push(four);
}else if(number === 9){
romanArray.push(nine);
}else if(number >= 5 && number < 9){
romanArray.push(five);
var remainder = number % 5;
for(var i = 0; i < remainder; i++) {
romanArray.push(one);
}
}else if (number > 0 && number < 4){
for(var e = 0; e < number; e++) {
romanArray.push(one);
}
}else{

}
return romanArray;
},


How can I improve this code?

• Your example confuses me. What would the variables four, nine, five, and one be? The names aren't very descriptive... Commented Dec 4, 2013 at 22:03
• You're really close to answering your own question with base_function(). Commented Dec 4, 2013 at 22:17
• I've updated the question per @DanielCook suggestion. The parameters four, nine, five, and one represent the strings at each position of the number. For example at last_number position ('IV', 'IX', 'V', 'I'), but at tens position the variables would be ('XL', 'XC', 'L', 'X'). Commented Dec 4, 2013 at 22:33
• @200_success I feel I'm close to solving this, but I don't know how to make the base function chain with the next function. Any and all help is greatly appreciated. Commented Dec 4, 2013 at 22:43

Normally, asking for code to be written is against the rules of this website. However, since you already have a working solution, an idea of where you want to go, and an independent solution from @Stuart, I suppose there's no harm in just finishing it for you:

hundreds: function(number) {
base_function(Math.floor(number / 100), 'CD', 'CM', 'D', 'C');
return this.tens(number % 100);
},

// etc.

• I was returning the wrong thing. I kept attempting to return this.base_function instead of this.tens. Good to know I was close thank you. Commented Dec 5, 2013 at 1:42
• I think you need to include this before base_function for the solution to work. Commented Dec 5, 2013 at 1:46

You may be aware that there are simpler answers to this problem, such as:

function toRoman(n) {
var r = '',
decimals = [1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1],
roman = ['M', 'CM', 'D', 'CD', 'C', 'XC', 'L', 'XL', 'X', 'IX', 'V', 'IV', 'I'];
for (var i = 0; i < decimals.length; i++) {
while (n >= decimals[i]) {
r += roman[i];
n -= decimals[i];
}
}
return r;
}


Bearing this in mind it might be useful to explain what exactly you are trying to achieve with your refactoring? E.g. is it to practice using more complicated language patterns?

• This is an elegant solution @Stuart. My goal was to refactor my code by eliminating repetition and using object oriented JavaScript(OOJS) for clarity. Commented Dec 4, 2013 at 23:06
• @ltrainpr my opinion, but "I want to do this using js prototype inheritence" is the source of quite a few crappy, unnecessarily complex frameworks I've seen people build over and over again. OO in JS is an awkward shim. It's useful for optimization, or if you're building something with a plugin architecture, but contributes little else. It's usually much better to rely on the functional aspects of JS than on the object-oriented ones /opinion Commented Dec 4, 2013 at 23:17
• My comment would also be that this problem does not call for OO design... There are no 'objects' here (except a number) and it does not in practice seem to improve clarity, and I can't see any way to improve the clarity and eliminate repetition while retaining the OO design. Commented Dec 4, 2013 at 23:18
• @GeorgeMauer You may be right but I don't think this problem would call for OO design in any language Commented Dec 4, 2013 at 23:20
• Except of course in Java where if you don't have an AbstractFactory that is driven by a xml configuration you're not doing it right :) All kidding aside, that's a good point. Commented Dec 4, 2013 at 23:21