2
\$\begingroup\$

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 :-)

\$\endgroup\$
  • \$\begingroup\$ Yes. My mistake. It's dummy code and I left it out by mistake. I corrected the original post. Good catch! \$\endgroup\$ – Sam Sep 12 '17 at 6:08
4
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Fantastic response! Really appreciate the explanations. You literally demonstrated the chaining of functions and using lots of short hand. Also avoided using temp variables. That's exactly what I was referring to. Based on your version of the code, my C# accent is not so subtle but I appreciate your being kind :-) Thanks again! \$\endgroup\$ – Sam Sep 12 '17 at 6:32
  • 1
    \$\begingroup\$ Glad you found it useful. In the future, feel free to wait a little while before accepting an answer, in case anyone else chimes in with something better! :) \$\endgroup\$ – BenC Sep 12 '17 at 6:44
  • \$\begingroup\$ @BenC nice answer. Could you please explain the machinery of the .filter(Boolean)? I see this the second time, so I assume this is idiomatic to JS, but I am still not sure how does it work under the hood. \$\endgroup\$ – Igor Soloydenko Sep 12 '17 at 19:12
  • 2
    \$\begingroup\$ @IgorSoloydenko like in C#, instead of lambda .filter(item => Boolean(item)), the method can be passed directly .filter(Boolean) \$\endgroup\$ – Slai Sep 12 '17 at 20:54
  • \$\begingroup\$ @Slai makes sense now. Thanks. I did not know that a) the compressed syntax without an arrow is meaningful; b) Boolean has a ctor with any as an argument. \$\endgroup\$ – Igor Soloydenko Sep 12 '17 at 20:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.