add tiny-js-world homework

[bmukha]

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

[OleksiyRudenko]

@bmukha good job!
Let's improve the code a bit.

[OleksiyRudenko]

Let's improve readability here.

Use Array#filter to skip properties where their names and/or values to do not meet criteria.

Use Array#map method to convert property names into property values.

if (i < props.length - 1)
^^ Not a good strategy. As a peer developer I am tasked to add 20 properties and move friends somewhere in the middle. To easy to make a mistake. In fact, you treat frends in a different way not because they are last property but because they are Array.isArray(variable) === true.

Glue them all with Array#join.

[bmukha]

Tnx for the suggestions. I just refactored the code according to them. Please, re-review.

[OleksiyRudenko]

@bmukha much better!
Still some room for improvements and opportunity to learn. Please check the comment below.

[OleksiyRudenko]

Do not change anything yet. At this stage please just answer questions and reflect on statements.

0. Describe in plain language (any but just plain) what would be the rules to print the value of any particular property?

1. Do we want to skip printing 0 only for hands? What if a character has 0 legs and 0 hands but 4 paws? What if I as a peer developer assigned to this project pass undefined instead of 0?

2. Array methods that return arrays are chainable.
   map and filter always do. reduce may return an array, thus chainable by other Array methods.
   This means that we can do filter(...).map(...).filter(...).map(...) or filter(...).filter(...).map(...) or map(...).filter(...).filter(...).map(...) etc.

3. The tinier the job a callback does the more readable and scalable the code is.
   e.g.

arr
  .filter(v => v !== 0)
  .filter(v => v !== "")
  .filter(v => Array.isArray(v) && v.length)

is cleaner and easier to extend and modify than
arr.filter(v => !(v=== 0 || v === "" || (Array.isArray(v) && v.length))

4. Probably, we want to discard not only zero values, but also undefined, null and "" (empty strings).
   Check what Boolean(v) does when v is either of 0, "", undefined, null, 111, "abc", [], [1,2,3].
   Check what is the result of [].join("whatever glue string").

5. Check what are the results of the statements below.

let v = 5;

v.length;
v?.length;

Explain.

6. How could you leverage the learnings to simplify data checks?

P.S. You may quote-reply and comment inline or refer to item numbers in your response. Please feel free to answer those one by one in separate comments and in any order.

P.P.S. We won't necessarily implement every learning.

P.P.P.S. Apologies for many updates. Just a really good case to dig deeper into the topic of data processing and validation.

[bmukha]

First of all, there is no need to apologise. I am here to learn and I appreciate your feedback a lot. Thank you very much for your time and effort.

0 and 1. I derived my decisions on what to filter out from those two rules:

if inhabitant has no hands then skip it or report 0 or undefined, it is totally up to you
Optional: each inhabitant can be friendly to 1 or more other inhabitants (or to none). If you implement this then the output should also list friends, i.e. human; John; male; 2; 2; Hello world!; Rex, Tom, Jenny

In the first rule, only hands were mentioned as questionable property. As for why I decided to filter them out only if hands === 0, well, I guess I really didn't think it through. From all options "skip it or report 0 or undefined" I've decided to go for the skip. And, in the second rule, if the friends array is empty, I decided to filter it out too for consistency. So, my interpretation was that only hands need to be checked, so that's why I did not go for some generalised solution.

2. I am aware of chaining methods, but it is still more readable for me as a novice to write code without it. But I got the point, should probably try to get used to it.

3. This one is really interesting to me. I'm not trying to argue, I'm trying to understand. Your way code loops over the array 3 times, while my way it loops only once. Genuine question: is it worth sacrificing a little bit of performance to achieve better readability? Should I always go with the readability approach, or does it depend on the situation?

4. Again, I am aware of what Boolean(v) does. But I can't think of a clean and elegant solution from the top of my head to filter everything I need to filter at once. While I can filter out all falsy values with it, the problem is empty array is still a truthy value.

5. I’ve done some research, and it looks like optional chaining. Totally new concept to me, since when I first tried to learn JS for my personal needs many years ago it wasn't even been introduced yet. I had to even update my Node from version 12 to 16 to make it work in my console, lol ) So, both examples should return undefined. But, if I understood everything correctly, the second one is useful when we need to check if some property exists and only then check its value.

6. So, to summarize it all: for each character, we need to filter out all the properties with falsy values and properties which are empty arrays. (I still can’t see if it could be done in one clean and concise check, though). Then we need to map through the remaining properties to join them together (and join non-empty array properties in the process). So, I guess, the code should look something like this:

characters.forEach((character) => {
  print(
    props
      .filter((prop) => Boolean(character[prop]))
      .filter((prop) => character[prop]?.length !== 0)
      .map((prop) =>
        Array.isArray(character[prop])
          ? character[prop].join(', ')
          : character[prop]
      )
      .join('; ')
  );
});

[OleksiyRudenko]

You are right, splitting one complex filter condition into consequent filter's with a simpler logic each affects performance. So, readability takes it cost (2 filters are slower by 30-50% compared with a single filter).

Consider logic as complex as (a || !(b[i] || c) && d[i][j]) || (e ? f[i+1]:g). Same refers to nested if/else's. Quite easy to make a mistake when need to change anything and a nightmare to debug.

If we can improve readability by other means we should do.

Your latest example contains two simple conditions, so we can already combine them both in a single filter condition with not much impact on performance and yet quite clean and readable.

One important observation is that now your code works with values only, as we do not need property checks anymore. Makes sense to convert property names into values on some very early stage and you need arrays converted to strings anyway. Check [].join('any delimiter'). The benefit: you reduce a number of data types to analyze and work with further down. A good chance to simplify value checks even further.

We also get rid of character[prop] on early stage and may use simple value further down the chain. Not much to save in this case but think of it as of a pattern for more complex cases.

In few words, simplify, minimize, reduce complex data structures and logic as early as possible.

If you feel confident as to the next code improvement iteration then go ahead. Feel free to ask further questions and sharing your ideas.

P.S. Do not use x?.prop here, stick to Array.isArray.

[bmukha]

One important observation is that now your code works with values only, as we do not need property checks anymore. Makes sense to convert property names into values on some very early stage and you need arrays converted to strings anyway.

Well, that was really helpful. By mapping first I can convert possible truthy empty arrays into falsy empty strings and then get rid of all the falsy values with one filter. Thank you very much for this suggestion! I just submitted refactored version, please, take a look.

[OleksiyRudenko]

@bmukha great job!