Wherefore art thou... Solved, Looking for Constructive Criticism

Hello! I am rather new at this, so I apologize in advance for any breach in etiquette or faux-pas.

After struggling for about a day and a half, maybe two, with JavaScript Intermediate Algorithm Scripting: Wherefore art thou Challenge, looking not only at the hints and previous lessons, but at outside resources as well, I finally reached a solution that satisfies all the requirements.

While I am ecstatic about having solved the challenge and mostly happy with the way my code looks. I would like to request, for someone more experienced than I to take a look at my solution and, if possible, critique my approach and code in general.

I included a link to the challenge as well as the code, formatted and in between spoiler tags.

Thank you very much for your time!

Challenge link


function whatIsInAName(collection, source) {
  var arr = [];
  // Only change code below this line
  let arrNew = [];
  let a = 0;
  let z = 0;
  let bool;

  let one = Object.keys(source);

  collection.filter(blue => selector(blue));    

  function selector(bine) {
     (bine[one[a]] === source[one[a]]) 
        ? arrNew.push(bine)
        : arrNew = arrNew;
  }

  while (a < one.length) {
     arrNew.filter(function(argh) {
         if (z < arrNew.length) {
           bool = true; 
        one.filter(function(growl){
          ((argh[growl] === source[growl]) 
              && argh.hasOwnProperty(growl)) 
                ? bool = bool
                : bool = false;
        });
        if (bool === true) { arr.push(argh);}
        z++
        }
     });
  
    a++
   };

  // Only change code above this line
  return arr;
}

 whatIsInAName([{ first: "Romeo", last: "Montague" }, { first: "Mercutio", last: null }, { first: "Tybalt", last: "Capulet" }], { last: "Capulet" });

Your code looks largely complicated for such an easy challenge… And the way you’re using .filter() methods is a big no-no-no! It should probably be forEach() instead.

You write code for a people to read, not for computer. Your code is your story and reading it I personally cannot get why array of keys of source object is named one.

1 Like

Thank you! I will back-track and look at replacing the stacked .filter() methods with forEach().

Code evolved with each test case. one is there, because at a point in time there was a two as well as a three.

Thank you again for your time and suggestions.
After reading your post I revisited my code and implemented some revisions.

function whatIsInAName(collection, source) {
  var arr = [];
  // Only change code below this line
  let arrNew = [];

  let chei = Object.keys(source);

  collection.map(blue => 
      (blue[chei[0]] === source[chei[0]])
          ? arrNew.push(blue)
          : arrNew = arrNew);
    
     arrNew.forEach(function(argh) {
        let bool = true;
        chei.forEach(growl =>
          ((argh[growl] === source[growl])
              && argh.hasOwnProperty(growl))
                ? bool = bool
                : bool = false);
         if (bool === true) { arr.push(argh);}
     });


  // Only change code above this line
  return arr;
}

 whatIsInAName([{ first: "Romeo", last: "Montague" }, { first: "Mercutio", last: null }, { first: "Tybalt", last: "Capulet" }], { last: "Capulet" });

  arrNew.forEach(function(argh) {
    let bool = true;
    chei.forEach(growl =>
      ((argh[growl] === source[growl]) &&
        argh.hasOwnProperty(growl)) ?
      bool = bool :
      bool = false);
    if (bool === true) {
      arr.push(argh);
    }
  });

In general, your variables names are not describing the data they represent. Variable names like bool, growl, argh are meaningless and do not make the code readable.

Also, in two places you are using the ternary operator in replace of a if/else statements, which is not its purpose and just adds to the code being more difficult to read.

Focusing just on the use of the ternary operator in the code above, it should be used when returning a value from an expression or assigning a value to a variable as shown below:

  arrNew.forEach(function(argh) {
    let bool = true;
    chei.forEach(growl => {
      bool = argh[growl] === source[growl] &&
        argh.hasOwnProperty(growl)
        ? bool
        : false
    });
    if (bool) {
      arr.push(argh);
    }
  });
2 Likes

Thank you very much, for your time and advice!

Thank you again for your time and suggestions.

Having come to realize that I will, hopefully, not be the only one reading thru the code that I write today, I have renamed some of the variables and will try to be more mindful thru the “baptism” process in the future. bool remained bool, it represents a boolean value, blue remained blue due to its, I feel, very localized usage.

Also modified the two uses of the ternary operator. The first one is now an if/else statement, and the second one was modified as per your example, which was enlightening. While curious about your statement regarding the purpose of the ternary operator, I do understand how it makes code more difficult to read.

Below, in between the spoiler tags is what the code looks like with the revisions made after reading your (@RandellDawson) comment as well as the comment @snigo wrote.


let arrNew = [];

  let chei = Object.keys(source);

  collection.filter(blue => { 
    if (blue[chei[0]] === source[chei[0]]) {
       arrNew.push(blue);
    } else { 
       arrNew = arrNew;
    }
  });


  arrNew.forEach(function(newit) {
    let bool = true;
    chei.forEach(keyit => {
      bool = newit[keyit] === source[keyit]
         && newit.hasOwnProperty(keyit)
            ? bool
            : false;
    });
    if (bool) { 
      arr.push(newit);
    } 
  });
    

I see you use filter or map often without using their returned value. Those are powerful tools, but if you don’t use their returned value it is a great misuse. Please use them appropriately or swap them for something else. (like forEach)

also, more descriptive variables would be a great improvement, like also a comment or two if needed for complex steps

1 Like

Thank you very much for your time and suggestions!