Coin Change Kata in ImmutableJS

I'm preparing for a job as a junior/intermediate JS developer. I'm comfortable with my ability to think in higher-order functions, but less confident in my syntax and style (I have more experience in Clojure, ClojureScript). This is simple but I welcome your comments.

import { Map } from 'immutable';

const coinChanger = (d, m) => { // d for denominations, m for money
  const denoms = d.sort((a,b) => b-a)
  const coinCounter = (coin, amount) => {
    const coins = Math.floor(amount / coin);
    const remainder = amount % coin;
    return Map({}).set(coin, coins).set("remainder", remainder);
  };
  const coinMap = denoms.reduce((result, coin) => {
    const amount = result.get("remainder"); 
    const coinCount = coinCounter(coin, amount); 
    return result.merge(coinCount);
  }, Map({"remainder": m}));
  return coinMap;
};

console.log(coinChanger([1, 5], 11));
console.log(coinChanger( [25, 10, 5, 1], 192 ));
console.log(coinChanger([25, 2], 105));

It looks like a nice implementation, but I don't think that using immutable.js is terribly necessary. JS already has a Map, which, while not immutable, could work too. However, a plain object might also be enough.

The slightly ironic bit is that despite choosing to use immutable data structures, your function actually has side-effects: sort modifies the receiver in-place, meaning that the d argument is mutated as a side-effect.

Inside the function itself, i.e. for local variables, I'd be less concerned about mutability. So for instance, a vanilla-JS solution using a plain object (since Map's API of get/set gets tiresome in a hurry) could be:

const coinChanger = (denominations, amount) => {
  return denominations
    .slice() // copy array before sorting
    .sort((a, b) => b - a)
    .reduce((result, value) => {
      result.coins[value] = (result.remainder / value) | 0; // bitwise trick to floor the result
      result.remainder %= value;
      return result;
    }, { remainder: amount, coins: {} });
};

Edit/sidenote: The | 0 trick, while neat, does have some very notable limitations. See the comments for a discussion (thanks, Thriggle!). Essentially, all numbers in JS are 64-bit floats, but behind the scenes, bitwise operations are performed on 32-bit ints. So the trick causes an unsafe cast to int, performs a pointless OR'ing with zero, and casts the result back to a float. So again: It's a trick. Here, given a large enough amount value, you will get weird results. So use with caution.

I've simplified it somewhat (removed the coinCounter function), and yes, the memo object for reduce is mutated. The effects are however contained within the reduce call, so... eh. The coin denominations are also coerced to strings when used as keys in the object, which a proper Map wouldn't do, but the coercion works both ways, so the keys can transparently be used as numbers later.

Of course, once the object is returned, only the caller holds a reference, and then it's the caller's responsibility – so that's where an immutable data structure might be better. But for the purposes of a quick alternative implementation, I'll leave things as-is.

Style-wise, your code looks fine. You're missing a semi-colon after sort (JS mostly works without semi-colons, but it looks like you're using them elsewhere, so stay consistent), and you could do with a little more whitespace here and there, but otherwise it's pretty good.

The only thing to wag a finger at, which Fanis Despoudis pointed out in a comment, is that d and m should be spelt out, rather than be explained in a comment. Especially because they're arguments, and they form the interface of the function, they should be readable. In general, all names should of course be clear and descriptive, but parameter and function names should be especially easy to parse, as that's what others will see first and make use of.

StackExchange Code Review Q#155967, answer score: 4