Sheetworker to Build Links and Skip Empty Values

I know enough to adapt others' code for my purposes, but I do not know enough yet to originate code. I have a sheetworker that is currently working, but I wonder if there is a better way to do this. It's building links that will be plugged into a Summary chat menu. The sheet has spaces for up to 6 specialties for these three skills. I only want links for the skills where a specialty is declared. I am assuming that the player will fill them in in order, so if specialty 2 is blank, I assume 3, 4, 5, and 6 are blank, too. 1) Is there a better way to do this that is efficient but clear to follow and update? My solution seems like a lot of code, but I can follow what is going on for the most part. 2) I would like to tweak this somewhat and have the display of the main skill name be capitalized (so "Craft" instead of "craft") in the label (the first ${s} reference in each "build =" line). I think what I need to do is make cppSkills into a different array, associating "craft" with "Craft," "perform" with "Perform," etc., but I'm not sure how to go about doing that. Thanks for any suggestions and advice! const cppSkills = ["craft", "perform", "profession"]; const cppIds = ["1", "2", "3", "4", "5", "6"]; const cppskillAttrs = _.flatten(_.map(cppSkills,(s)=>_.map(cppIds, (i)=>`${s}${i}_name`))); const cppskillEvents = _.map(cppskillAttrs,(a)=>`change:${a}`); on(`${cppskillEvents.join(' ')}`, function() { getAttrs([...cppskillAttrs], (value) =>{ let update = {}; _.each(cppSkills, (s)=>{ if (value[`${s}1_name`] != "") { build = `[${s} (@{${s}1_name})](~${s}1_check)`; if (value[`${s}2_name`] != "") { build = build + `[${s} (@{${s}2_name})](~${s}2_check)`; if (value[`${s}3_name`] != "") { build = build + `[${s} (@{${s}3_name})](~${s}3_check)`; if (value[`${s}4_name`] != "") { build = build + `[${s} (@{${s}4_name})](~${s}4_check)`; if (value[`${s}5_name`] != "") { build = build + `[${s} (@{${s}5_name})](~${s}5_check)`; if (value[`${s}6_name`] != "") { build = build + `[${s} (@{${s}6_name})](~${s}6_check)`; } } } } } } update[`${s}var`] = build; }); setAttrs(update); }); });

Do you need all the if statements to be nested like that? It makes sense (kind of...) to do it that way only if when you have a "" every skill after it is also "". But if you can have an "" followed by a non-empty skill, the above structure will miss some skills.

Thanks, GiGs. Yes, the assumption I make is the entries are filled in in order on the sheet. It made my coding easier to understand, but yes, it contributes to my sense that there must be an easier/shorter way to do this. I could build another each loop and use 1 through 6 as variables, but then I would loop through all of them every time. OTOH, that's only 18 entries to check, so maybe I am making this less efficient by trying to be more efficient? :-) This is part of what I have little sense of, and I am trying to get a handle on. Where is the line of balance between one method and the other? I know there is no set easy answer, and it's more you have to get a sense from experience, which I am (slowly) building.

You should never make assumptions where users are concerned :) If the program doesnt have a way to enforce users to act in the way you expect, assume that sometimes they will do what you dont expect. Here's how I'd do it. First notice i swapped several of the functions fromn underscore to the javascript native versions, since they are now supported natively. I think they should still work fine - I've never used underscore's flatten so thats the first line I'd check if things dont work properly. Also note i tend to prefer setting words instead of single letters for variable names, especially where they rae used over and over, because it makes it a lot more readable. const cppSkills = ["craft", "perform", "profession"]; const cppIds = ["1", "2", "3", "4", "5", "6"]; const cppskillAttrs = _.flatten(cppSkills.map(skill => cppIds.map(index =>`${skill}${index}_name`))); const cppskillEvents = cppskillAttrs.map(a =>`change:${a}`).join(' '); // added a function for the creation of the string, so you don't need to rewrite it every time const buildname = (skill, index, value) => value ? `[${skill} (@{${skill}${index}_name})](~${skill}${index}_check)` : ''; on(cppskillEvents, function() {     getAttrs(cppskillAttrs, (value) =>{         let update = {};         cppSkills.forEach(skill => {             let build = '';             cppIds.forEach(index => {                 build += buildname(skill, index, value[`${skill}${index}_name`]);             });             update[`${skill}var`] = build;         });         setAttrs(update);     }); });

Yes, looping inside looping can be inefficient, but when you know the sizes of the loops its usually fine. Loops can do thousands of items without issue, and here we only have 18. The reduce function would be more efficient i think, but the amount is negligible here and forEach syntax is much easier.

Thanks, GiGs. I can follow that! :-) Not to abuse your help, but any thoughts to my second question (having a capitalized label for the skill)? I am guessing it would be another parameter to add to the function but how to associate Craft with craft? Thanks again for your help!

Oh right, I missed that. is the only place it needs to be changed the first skill in this section? `[${skill} (@{${skill}${index}_name})](~${skill}${index}_check)` Edit: if so, change the buildname function to this const capFirstLetter = word => word.substr(0, 1).toUpperCase() + word.substr(1).toLowerCase(); const buildname = (skill, index, value) => value ? `[${capFirstLetter(skill)} (@{${skill}${index}_name})](~${skill}${index}_check)` : '';

Right. The label that will display in the chat button. Edit: Even better! A command to just change the case as opposed to introducing another parameter. Brilliant! Thanks again!

You replied too quickly :) see my edit.