0
\$\begingroup\$

I have written a small program to convert numeric cardinal numbers (i.e, 4, 35) into Spanish. Currently, it only supports numbers 1-99, however I would like to increase it to support numbers of any length.

Before I do this, I am seeking some feedback on my code structure. I feel that I have too many if statements and do not abstract my code enough, and I do not wish for it to be an elongated nightmare of if statements. I am also unsure how I should split up numbers of different sizes, and what variable names to use.

getSpanishCardinal = cardinal => {
  if (0 <= cardinal && cardinal <= 15) {
    return getCardinals()[cardinal];
  }
  if (16 <= cardinal && cardinal <= 19) {
    return sixteenToNineteen(cardinal);
  }
  if (cardinal >= 20 && cardinal <= 99) {
    return twentyToNinetynine(cardinal);
  }
};

twentyToNinetynine = cardinal => {
  if (cardinal % 10 == 0) {
    return getCardinals()[cardinal];
  }
  if (21 <= cardinal && cardinal <= 29) {
    return "veinti" + getSpanishCardinal(rightMostDigit(cardinal)).toString();
  }
  return (
    getCardinals()[leftMostMultiplier(cardinal)].toString() +
    " y " +
    getSpanishCardinal(rightMostDigit(cardinal)).toString()
  );
};

sixteenToNineteen = cardinal => {
  let rightMost = rightMostDigit(cardinal);
  return "dieci" + getSpanishCardinal(rightMost).toString();
};

leftMostMultiplier = cardinal => {
  let newDigit = cardinal.toString()[0];
  const cardinalString = cardinal.toString();
  const length = cardinalString.length;
  for (let i = 1; i < length; i++) {
    newDigit += "0";
  }
  return parseFloat(newDigit);
};

rightMostDigit = cardinal => {
  return cardinal % 10;
};

getCardinals = () => {
  return {
    0: "cero",
    1: "uno",
    2: "dos",
    3: "tres",
    4: "cuatro",
    5: "cinco",
    6: "seis",
    7: "siete",
    8: "ocho",
    9: "nueve",
    10: "diez",
    11: "once",
    12: "doce",
    13: "trece",
    14: "catorce",
    15: "quince",
    20: "veinte",
    30: "treinta",
    40: "cuarenta",
    50: "cincuenta",
    60: "sesenta",
    70: "setenta",
    80: "ochenta",
    90: "noventa",
    100: "cien",
    200: "doscientos",
    300: "trescientos",
    400: "cuatrocientos",
    500: "quinientos",
    600: "seiscientos",
    700: "setecientos",
    800: "ochocientos",
    900: "novecientos",
    1000: "mil",
    1000000: "millón"
  };
};

console.log(getSpanishCardinal(56));
\$\endgroup\$
1
  • \$\begingroup\$ Hint: Any numeric literal in english is the same, except with "," and "." reversed to be "." and ","; As such, 1,000.4 is 1.000,4 \$\endgroup\$ Jan 6, 2020 at 18:05

1 Answer 1

1
\$\begingroup\$

The function declarations are at least lacking a const statement (or let or var if you prefer):

const getSpanishCardinal = cardinal => {

However I would suggest to using regular function statements instead.


The getCardinals function is a bit pointless. A simple constant instead would make more sense:

const CARDINALS =  {
  0: "cero",
  1: "uno",
  // ...
}

For readability I'd also suggest to split it up into separate objects for 0 to 15, the tens, the hundreds, etc.

This would also allow to assign assign the tens to the base number e.g.:

const TENS = {
     2: "veinte",
     3: "treinta",
     4: "cuarenta",
     // ...
}

and then you don't need to "build" numbers in leftMostMultiplier:

const twentyToNinetynine = cardinal => {
  // ...
  return (
    TENS[cardinal / 10] +
    " y " +
    getSpanishCardinal(rightMostDigit(cardinal))
  );
};

(BTW, there seem to be a lot of unnessecary toString()s.)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.