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

JavaScript
1

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" });
2

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
3

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.

4

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" });
6

Thank you very much, for your time and advice!

7

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 (@camperextraordinaire) 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);
    } 
  });
8

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
9

Thank you very much for your time and suggestions!