Receive an array input and output a string

Question

I have some code which is working, but I would be interested if anyone could review it and tell me if there is a more efficient way of achieving my goal.

I receive input from a data source as an array such as ["Y","Y",,"Y","Y",,"Y"].

The values relate to investment fund names and I am outputting a string such as "Balanced, Cash, High Growth, Moderate and Growth options".

If there is only one value populated with "Y" the string could be "Conservative option".

If there are two values, the string could be "Conservative and Cash options".

For more than two values, the string could be "Conservative, Cash and Moderate options".

I know that the order of the values will always be the same, eg the first value will always be the Balanced option. Here is the code:

 // balanced, cash, high growth, moderate and growth var params = ["Y","Y",,"Y","Y",,"Y"]; getString(params) function getString(values) { // map for the values var fundMap = { 0: "Balanced", 1: "Cash", 2: "Conservative", 3: "High Growth", 4: "Moderate", 5: "Shares", 6: "Growth" } var fundArray = []; // get fund names from map and push to array for (var i = 0; i < values.length; i++) { if (values[i] == "Y") { fundArray.push(fundMap[i]); } } console.log(fundArray); var fundString = ""; if (fundArray.length == 1) { fundString = fundArray + " option"; } else if (fundArray.length == 2) { fundString = fundArray[0] + " and " + fundArray[1] + " options"; } else { for (var i = 0; i < fundArray.length -2; i++) { fundString = fundString + fundArray[i] + ", "; } fundString = fundString + fundArray[fundArray.length -2] + " and "; fundString = fundString + fundArray[fundArray.length -1] + " options"; } console.log(fundString); } 

I would love to know if there is a more efficient or just a neater way of writing this code please.

Thank you!

Answer 1

A few stylistic nitpicks: the function body should be indented and there should be a space following the - signs.

getString should be changed to take an array of boolean values, instead of a sparse array of "Y" strings. You can write a separate function to convert your original format to a bool array.

function optionsToBoolArray(array){ var converted = []; for(var i = 0; i < array.length; i++){ converted[i] = array[i] === "Y"; } return converted; } 

getString is also a undescriptive name for your function. You should change it to something along the lines of optionsToReadableString.

fundMap doesn't need to be an object. Since you are currently using like an array, explicitly make it an array for clarity.

fundString = fundArray + " option"; should be fundString = fundArray[0] + " option"; to avoid implicit conversions for readability.

The code for multiple options can also be simplified by using array methods:

else { fundString = fundArray.slice(0,fundArray.length - 1).join(", ") + " and " + fundArray[fundArray.length - 1] + " options"; } 

This also lets you eliminate the else if branch.

You should also extract the console.log out of the function to separate the logic. After doing this, you can eliminate fundString completely by just returning the value from each branch of the if/else.

You should also look into learning ES6 features such as using let instead of var.