DEV Community

Cover image for Let's Solve: Code Challenge - Picking Numbers

Let's Solve: Code Challenge - Picking Numbers

Ryan on October 21, 2017

Hey hey, welcome to my first Let's Solve! We're going to be solving an algorithm code challenge called Picking Numbers, which is categorized as an ...
Read full post
Collapse
 
tobias_salzmann profile image
Tobias Salzmann • • Edited

I like the approach! Maybe a bit of feedback:

The necessity of using comments to structure code is often an indication for a function being too long. Your code is already well structured, so you can directly extract the parts of your code as functions:

function main() {
    var numbers = readInput();
    var numberCounts = constructAndPopulateMap(numbers);
    var maxSum = findMaxSumOfAdjacentNumbers(numberCounts);

    console.log(maxSum);
}

function readInput() {
  var n = parseInt(readLine());
  a = readLine().split(' ');
  a = a.map(Number);
  return a;
}

function constructAndPopulateMap(numbers){
  var map = new Array(100);
    map.fill(0);

  for(var i = 0; i < a.length; i++){
        map[a[i]]++;
  }

  return map;
}

function findMaxSumOfAdjacentNumbers(map) {
  var max = 0;
  for(var i = 1; i < map.length; i++){
    if(map[i] + map[i - 1] > max){
      max = map[i] + map[i - 1];
    }
  }
  return max;
}
Enter fullscreen mode Exit fullscreen mode

This has several benefits:

  • no comments needed
  • Variables are scoped appropriately.
  • You can easily test the parts of your program.
  • Improved readability

Here's a solution (minus the parsing) using exclusively functions from the utility library ramda. Each number is assigned to 2 potential clusters and all that's left is to find the most common cluster.
There is quite some functional lingo in there, but it doesn't require much javascript knowledge.

const largestCluster = (numbers) => {
  const countByValue = reduceBy(inc, 0)(identity)
  const maxOccurences = pipe(countByValue, values, reduce(max, 0))

  const clusters = chain(n => [[n-1, n], [n, n + 1]])(numbers)
  return maxOccurences(clusters)
}
Enter fullscreen mode Exit fullscreen mode

repl:
ramdajs.com/repl/?v=0.25.0#?const%...

Collapse
 
nektro profile image
Meghan (she/her) • • Edited

Great job on solving the problem!

Just some pointers for future reference:

  • new Array(100) is pretty wasteful, but I'll get back to this in a sec
  • map.fill(0) in your code isn't doing anything because you don't assign the return value back to map. Array.prototype.fill is not a modifier function
  • the entire first few lines could be condensed to
  let map = readLine().split(' ').map((x) => {
      return parseInt(x) + 1;
  });

JavaScript can be pretty confusing at time but I hope you stick around and again nice work :D

Collapse
 
ryhenness profile image
Ryan • • Edited

Hey Sean, thanks for the response! I think for the way that I'm approaching the problem, that I actually need to initialize the array with 100 indices - otherwise, I couldn't map the keys to their array index positions. Also, I think fill does modify array values, as well as return the outcome of the fill. I tested that out on JSFiddle.

(Your avatar is dope btw. :))

Collapse
 
rattanakchea profile image
Rattanak Chea • • Edited

The solution and explanation are great.

var n = parseInt(readLine());
    a = readLine().split(' ');

should be using , instead of ;. And make use of a meaning name. I would also look into making use of built-in Map in ES6.

var n = parseInt(readLine()),
    numbers = readLine().split(' ');


`

Collapse
 
paulupendo profile image
candy_man •

I love the way you approached the problem. Shows how well you understand the techniques you opted to use.

Collapse
 
ryhenness profile image
Ryan •

Thanks man! :) I appreciate that!

Collapse
 
wnds profile image
wnds • • Edited

Ryan - absolute difference in problem statement should be changed to difference only as this solution of using array indexes does not work when we have negative numbers in the input array.