JS exercise to process an array of animals

Question

My background is primarily in C#. Though, I've gotten comfortable with JavaScript, I feel that stylewise my JS code looks more like C# code.

Does the following code's style conform to "standard JS style"?

processArray = (myArray) => {

   if(typeof myArray === "undefined" || myArray == null || myArray.length == 0)
      return null;

   let animals = [];
   let animal = {};

   for(let item of myArray) {

      animal = getAnimal(item);
      if(item !== null)
         animals.push(animal);
   };

   return animals;
}

getAnimal = (data) => {

   if(typeof data === "undefined" || data == null)
      return null;

   const animal = {
      id: data.id,
      type: data.type,
      name: data.name
   };

   return animal;
}

It's important to note that I'm not so much talking about formatting such as indentations, opening curly braces on the same line as the function name, etc.

What I'm really talking about is the way the code is structured. For example, as an "old school" C# developer, I like creating variables and assigning my data to them and cleanly returning them in my methods.

I see that a lot of JS developers chain one function to another or use a lot of callbacks. In other words, I see a lot of intermingling of functions. I think C# has also borrowed a few patterns from JS but for the most part, the coding style I used here is pretty typical "old school" C#.

Maybe an analogy will help clarify what I'm talking about. It's like a person whose native language is German/Italian/Japanase/etc. speaking English with an accent. Even if the grammar and even choice of words are perfect, the accent always gives it away.

So, I'm trying to understand if I'm writing JS code with a C# accent :-)

Answer 1

I do spot a minor "C# accent" :)

Unless you are dealing with very unruly data, an idiomatic approach would likely drop most of the special-case checks and use map instead of the for-loop:

// This function could even be removed, depending on the larger details of your code
function getAnimals(data) {
  return data.map(getAnimal);
}

function getAnimal(item) {
  return {
    id: item.id,
    type: item.type,
    name: item.name,
  };
}

If the source array may contain null-ey items, you could filter them out beforehand, instead of after attempting to process them:

function getAnimals(data) {
  return data
    .filter(Boolean)
    .map(getAnimal);
}

(null or undefined will evaluate to false in a boolean context.)

If you have a utility library like Lodash on hand, you could use a function like pick to further simplify things:

const getAnimal = item => _.pick(item, ['id', 'type', 'name']);

Otherwise, on a more general scale, most JS code tends to omit explicit null/undefined checks. Let the code throw an error if necessary, otherwise fix whatever is calling your function, eg. by filtering. There's a place for such checks, but in JS it's usually not in every function.

What I'm really talking about is the way the code is structured. For example, as an "old school" C# developer, I like creating variables and assigning my data to them and cleanly returning them in my methods.

I don't think the code you've provided here merits any temporary variables. Idiomatically, it's a single function applied via Array#map.

---

Minor syntax things

• Your functions are currently in the global namespace, because JS permits variables with no qualifiers as implicit globals. Prefix them with const or let to explicitly make them variables, or use function when possible since it plays nicer with hoisting:

  const processArray = (myArray) => {...
  // or, as above
  function processArray(myArray) {...
  

• Many people omit the braces around the single argument of an arrow function. It's optional, but reads slightly cleaner:

  const processArray = myArray => {...
  

• myArray is a toy name, I am assuming this is a toy example

• Always use === for comparisons, which you do, unless you want to compare against both null and undefined, which you do, in which case you can do myArray == null. To go a bit further, your first conditional would more simply be written:

  if (!myArray || !myArray.length) {
  

  since null and undefined are both "falsey", and so is a zero length.

Essentially: your code works fine -- but it could be a lot shorter.