4
\$\begingroup\$

Yesterday you found some shoes in your room. Each shoe is described by two values:

  • type indicates if it's a left or a right shoe;
  • size is the size of the shoe.

Your task is to check whether it is possible to pair the shoes you found in such a way that each pair consists of a right and a left shoe of an equal size

For:

shoes = [[0, 21], 
       [1, 23], 
       [1, 21], 
       [0, 23]]

the output should be true;

For:

shoes = [[0, 21], 
       [1, 23], 
       [1, 21], 
       [1, 23]]

the output should be false.

So I wrote the code below,added some comments

 shoes = [[0, 21], 
           [1, 23], 
           [1, 21], 
           [0, 23]];
           
  function pairOfShoes(shoes) {
        //replace 0 with -1 so I can reduce them to 0 later if they are a pair
        for (var i = 0; i < shoes.length; i++) {
            if (shoes[i][0] == 0) {
                shoes[i][0] = -1
            }
        }
        //reduce the array to object with shoe size as key
        function objectify(array) {
            return array.reduce(function(p, c) {
                p[c[1]] = (p[c[1]] != undefined ? p[c[1]] : 0) + c[0];
                return p;
            }, {});
        }
    
        var p = objectify(shoes);
        //check for each shoe size if the value is 0
        for (var x in p) {
            if (p[x] != 0) {
                return false;
            }
        }
        return true;
    }         
    
 console.log(pairOfShoes(shoes));   

Seems convoluted to me can this code be improved?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Two things I would say. (1) The use of names like p and c make it hard to understand. Use more descriptive names and assign intermediate variables (let size=c[1]) to make it easier to understand. (2) Try and keep all your code in a function at the same level. If you are going to use a function objecttify then add a function to changeLeftFlag and another to checkAllEntriesAreZero \$\endgroup\$ Mar 16 '17 at 16:53
2
\$\begingroup\$

Convert your test values where you need them

Your initial for loop only sets each "right / left" value to -1 if it is 0. This can be done in the reduce function itself. Also, your conditional p[c[1]] != undefined is unnecessary, as undefined evaluates to false, and there is no concern here about other "falsy" values such as zero.

Use Javascript's array methods

  • .some() can eliminate unnecessary for loops with conditionals in Javascript.

  • .values() lets you concisely iterate through values of an object.

Combined with the arrow operator =>, these can result in concise code that reads a little more like a natural sentence.

Applying both of the above points to your code:

shoes = [[0, 21], 
         [1, 23], 
         [1, 21], 
         [0, 23]];

function pairOfShoes(shoes) {
    var p = shoes.reduce(function(p, c) {
        p[c[1]] = (p[c[1]] || 0) + (c[0]==1 ? 1 : -1);
        return p;
    }, {});
    if (Object.values(p).some(x => x != 0)) { return false }
    else { return true };
}        

console.log(pairOfShoes(shoes));

[Edit: It might also be clearer to replace your reduce function with a `for' loop.]

\$\endgroup\$
5
  • \$\begingroup\$ I tested you answer it gives me false... \$\endgroup\$
    – Mihai
    Mar 16 '17 at 11:32
  • \$\begingroup\$ I'm still editing the dumb thing. \$\endgroup\$
    – Ryan Mills
    Mar 16 '17 at 11:36
  • \$\begingroup\$ Done. And JSfiddle says it's all good. \$\endgroup\$
    – Ryan Mills
    Mar 16 '17 at 11:44
  • \$\begingroup\$ Apparently Object.values is poorly supported on mainstream browsers, on Codewars I had to replace it with var vals = Object.keys(p).map(function(key) { return p[key]; But it works,thanks for the effort. \$\endgroup\$
    – Mihai
    Mar 16 '17 at 11:57
  • \$\begingroup\$ Yeah it's probably the arrow function. That's new in ES6/ES2015. \$\endgroup\$
    – Ryan Mills
    Mar 16 '17 at 12:05
1
\$\begingroup\$

Ryan's answer is great (and already accepted). This is just to show a few tricks:

const allPairs = (shoes) => {
  const pairings = shoes.reduce((pairings, shoe) => {
    const [type, size] = shoe;
    pairings[size] = (pairings[size] || 0) + (type ? +1 : -1);
    return pairings;
  }, {});
  return !Object.values(pairings).some(value => value);
};

The above uses the const keyword, rather than var, as it's generally better to use the stronger declaration when possible.

It also uses array destructuring to assign type and size, which makes the code a little clearer than using numeric indices.

Finally, rather than writing an if..else for the return values, it just negates the result of some and returns it directly; it's already a boolean. It also skips comparing to zero in the some callback; zero is falsey already.

\$\endgroup\$

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.