Push string to array if the array you want to push from is undefined

Question

I am not a coder, I am messing around with some JavaScript as part of modding a game, so bear with me. This game supports es5/everything Chromium 28 supported.

I had code which pushed a string to an array from a variable, and when the variable was undefined a fixed string was pushed instead:

       slotsArray.push({
          landing_policy: ai.landing_policy || 'no_restriction'
        });

The setup changed such that where ai.landing_policy was set it would contain multiple values, so it become an array. When it wasn't set only a single entry was required.

The same code does not appear to work where an array is in place:

        for (var i = 0; i < count; i++) {
          slotsArray.push({
            landing_policy: ai.landing_policy[i] || 'no_restriction'
          });
        }

An error is produced because it's trying to check a value from a variable that hasn't been defined. I expected that to cause it to use the fixed value, but apparently that's not what happens, it just fails.

I've changed my approach to the code seen below in full:

      if (Array.isArray(ai.landing_policy)) {
        for (var i = 0; i < count; i++) {
          slotsArray.push({
            landing_policy: ai.landing_policy[i]
          });
        }
      }
      else {
        slotsArray.push({
          landing_policy: ai.landing_policy || 'no_restriction'
        });
      }

This code works, but what I'm looking to understand is whether this was the best solution? The old method felt elegant, while the new one looks a little clumsy.

What is count, is it ai.landing_policy.length? Even if not, IMO your current code looks fine, its intent is quite clear and I don't think there's any clean way to make it more DRY. Also note that since you're using Array.isArray, that's an ES6 method (and being able to use ES6 is great) — CertainPerformance
– CertainPerformance, Commented Aug 17, 2019 at 20:52
@CertainPerformance var count = ai.copies || 1; is used to set the size of the loop, though in retrospect I could just set the loop by the size of the array given the two values will always be the same (the 1 is a holdover from the failed approach). I'm surprised the es6 function works (I didn't know it was es6) as navigator.appVersion.match(/.*Chrome\/([0-9\.]+)/)[1] returned "28.0.1500.68", which my understanding is es5, so there must be more going on under the hood than I understand. — Quitch
– Quitch, Commented Aug 17, 2019 at 21:37
I had posted an answer using map and ternary operator before realizing your code is much more readable. I'd keep it as it is. — adiga
– adiga, Commented Aug 17, 2019 at 21:42
Flagged the question as opinion-based; There isn't a code-problem you are trying to solve. You should ask this on CodeReview: codereview.stackexchange.com — Rob Monhemius
– Rob Monhemius, Commented Aug 17, 2019 at 23:44

FZs · Accepted Answer · 2019-08-18 04:56:56Z

1

You can use the ternary operator(? :).

It will return the second value if the first is true, and the third otherwise.

I've used array instanceof Array instead of Array.isArray(array) to support ES5.

var isArray = ai.landing_policy instanceof Array
for (var i = 0; i < (isArray ? count : 1); i++) {
    slotsArray.push({
        landing_policy: isArray ? ai.landing_policy[i] : ai.landing_policy || 'no_restriction'
    });
}

edited Aug 18, 2019 at 4:56

answered Aug 17, 2019 at 20:53

FZs

18.8k13 gold badges47 silver badges56 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

CertainPerformance Over a year ago

This will push items from 0 to count many times to the array regardless, which may not be desirable. Not sure what count is, though...

FZs Over a year ago

@CertainPerformance Good catch, I've reworked my solution, thanks!

Quitch Over a year ago

Thank you, this is helpful. If I'm understanding this correctly (after reading your link), this code checks whether ai.landing_policy is an array, then sets isArray to true or false. Then later on ? is used to evaluate whether isArray is true or false, and if true uses the first expression and if false uses the second expression. But my current code also supports landing_policy being a string, which I don't believe this code block allows for, correct? Also, how does it know which entry from the array to draw from without adding [i] to ai.landing_policy?

FZs Over a year ago

@Quitch I've tried to answer as quick as possible, and... as you see, I made some mistakes. You have to add [i] (as you've thought). And I've also corrected the support for string. See my answer now!

iwaduarte · Accepted Answer · 2019-08-18 05:06:14Z

1

Elegant solution not always converse to the most readable/desirable. I would probably do something like:

 const formattedPolicy = ai.landing_policy.map(policy => policy || 'no_restriction');
 slotsArray = [...formattedPolicy ];

Course this has to imply that the ai.landing_policy is always an array. If you need to double check first you could also do:

const formattedPollicy = ai.landing_policy.constructor === Array 
               ? ai.landing_policy.map(policy => policy || 'no_restriction');
               : [ai.landing_policy]

Looks like an elegant or short imho but your code is way more readable.

answered Aug 18, 2019 at 5:06

iwaduarte

1,74919 silver badges26 bronze badges

1 Comment

Quitch Over a year ago

Thanks. As I’m not a coder myself (beyond some Pascal way back in school) I don’t really have experience with self documenting code, or the idea of what is readable vs what is not, so a lot of my thinking has just been driven by how can I do what I want to do in the least amount of code possible. I appreciate the thoughts.

Collectives™ on Stack Overflow

Push string to array if the array you want to push from is undefined

2 Answers 2

4 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related