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!

• CertainPerformance: You have a sparse array here: var params = ["Y","Y",,"Y","Y",,"Y"]; Sparse arrays are almost always a mistake, consider fixing the data source to use a well-formatted array instead, if possible
• Malcolm Whild: Thank you for this tip - I hadn't heard of the term "sparse array" before and I'm still not sure why it's to be avoided. Regarding the data source - yes I could fix that as I'm exporting it from Access, but that would require writing queries in Access which is what I'm trying to avoid as they have to be run manually. In addition, we often get data supplied to us as csv so in those instances I have no control over the input at all.

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.

• Malcolm Whild: Cool - thank you! That's just the kind of thing I was looking for. Gonna play with it this afternoon when my little boy's asleep :-)
• Malcolm Whild: OK - I've managed to confuse myself pretty well :-) so here are a couple of questions: Array.prototype.map - is this an overall term for array.map(function)? After the function to get a boolean array I tried to use let fundArray = array.map(function(boolValue, i) { return boolValue ? fundMap[i] : array.splice(i, 1); }); return fundArray; I wanted to remove those elements where boolValue === false but I see that it messes with the index i so I'd have to write more code to fix that. Does it then mean that I would actually be writing more code than the original for loop?
• SuperStormer: On second thought, the string conversion code is fine. map would probably introduce more complexity. Also, any something.prototype.something functions are just methods you can call on any instance of an object, including map for arrays.
• Malcolm Whild: Thank you so much for your help. I posted the completed code jsfiddle.net/malcolmwhild/fzhwc13o